Skip to content

cli::test_utils: Remove allow(clippy::arithmetic_side_effects) #933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilya-bobyr merged 1 commit into from
Apr 19, 2024
Merged

cli::test_utils: Remove allow(clippy::arithmetic_side_effects) #933

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions cli/src/test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(clippy::arithmetic_side_effects)]

use {
solana_rpc_client::rpc_client::RpcClient,
solana_sdk::{
Expand Down Expand Up @@ -46,15 +44,15 @@ pub fn wait_n_slots(rpc_client: &RpcClient, n: u64) -> u64 {
loop {
sleep(Duration::from_millis(DEFAULT_MS_PER_SLOT));
let new_slot = rpc_client.get_slot().unwrap();
if new_slot - slot > n {
if new_slot.saturating_sub(slot) > n {
Copy link

@joncinque joncinque Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally aside, but I never realized this is technically wait_n_plus_one_slots

Copy link
Author

@ilya-bobyr ilya-bobyr Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a bug.
It is only used in the CLI tests and from what I can understand from the tests, they do not expect it to be n + 1.

One set of tests wait one slot to work around the program deployment limitation. 1 slot is enough there.

The second set of tests is for staking.
I do not understand staking all that well, but here is a usage example:

    // to `stake2_keypair`
    check_balance!(rent_exempt_reserve, &rpc_client, &stake_keypair.pubkey());
    check_balance!(50_000_000_000, &rpc_client, &stake2_keypair.pubkey());

    // wait for new epoch plus reward blocks
    wait_for_next_epoch_plus_n_slots(&rpc_client, 1);

Let me send a PR to s/>/>=/.

Copy link

@joncinque joncinque Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The staking example should behave correctly when waiting just one slot, since rewards payout is meant to take one block with partitioned rewards

return new_slot;
}
}
}

pub fn wait_for_next_epoch_plus_n_slots(rpc_client: &RpcClient, n: u64) -> (Epoch, u64) {
let current_epoch = rpc_client.get_epoch_info().unwrap().epoch;
let next_epoch = current_epoch + 1;
let next_epoch = current_epoch.saturating_add(1);
println!("waiting for epoch {next_epoch} plus {n} slots");
loop {
sleep(Duration::from_millis(DEFAULT_MS_PER_SLOT));
Expand Down