Skip to content
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.

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

Add fn add_fee_input() and make fn add_random_fee_input() truly random #829

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

rafal-ch
Copy link
Contributor

@rafal-ch rafal-ch commented Sep 17, 2024

Previously the add_random_fee_input() wasn't actually producing a random input, which was confusing. This PR:

  1. adds the add_fee_input() which is an equivalent to the former version of add_random_fee_input()
  2. modifies the add_random_fee_input() to accept an rng and provide truly random fee input

For compatibility reasons, all tests that were previously using add_random_fee_input() were modified to use add_fee_input().

This change should make the test code more explicit and make it easier to create random transactions in test code.

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

Follow-up draft PR in fuel-core: FuelLabs/fuel-core#2211

@rafal-ch rafal-ch marked this pull request as ready for review September 17, 2024 14:57
@rafal-ch rafal-ch added this pull request to the merge queue Sep 17, 2024
}

#[cfg(feature = "rand")]
pub fn add_fee_input(&mut self) -> &mut Self {
Copy link
Contributor

@acerone85 acerone85 Sep 17, 2024

Choose a reason for hiding this comment

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

can this be simply

self.add_rnadom_fee_input(rand::rngs::StdRng::seed_from_u64(2322u64))

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would work. Since it's not critical, I'll consider updating this part with next PR to fuel-vm.

@acerone85
Copy link
Contributor

one minor comment, but otherwise LGTM

Merged via the queue into master with commit 4796c87 Sep 17, 2024
39 checks passed
@rafal-ch rafal-ch deleted the update_add_random_fee_input branch September 17, 2024 15:27
@xgreenx
Copy link
Collaborator

xgreenx commented Sep 26, 2024

@rafal-ch This change is breaking, so it should modify the changelog

@rafal-ch
Copy link
Contributor Author

This change is breaking, so it should modify the changelog

I created a follow-up PR: #836

@xgreenx xgreenx mentioned this pull request Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Skips the CI changelog check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants