Skip to content

chore: test contract deployment with salt #1658

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

hal3e
Copy link
Contributor

@hal3e hal3e commented May 13, 2025

closes: #1181

Checklist

  • All changes are covered by tests (or not applicable)
  • All changes are documented (or not applicable)
  • I reviewed the entire PR myself (preferably, on GH UI)
  • I described all Breaking Changes (or there's none)

@hal3e hal3e self-assigned this May 13, 2025
@hal3e hal3e requested a review from a team as a code owner May 13, 2025 10:35
@@ -46,6 +46,45 @@ async fn test_multiple_args() -> Result<()> {
Ok(())
}

#[tokio::test]
async fn contract_deployment_with_salt() {
Copy link
Member

@MitchTurner MitchTurner May 26, 2025

Choose a reason for hiding this comment

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

Can we reformat this to match our testing format standard:
https://www.notion.so/fuellabs/Engineering-Style-Guide-83ba6fd81f12443796c814ecfc746b39?pvs=4#1742f2293f3180f0931bd1355b0afa13

It looks like the setup_program_test! macro is obscuring some of this, so maybe just

  1. rename the test to clarify what we are doing and what we are expecting: sut__when..._then...
  2. for the given/when/then, maybe just put the //given and //when above the macro and the //then above the asserts

I don't really know what this test is doing other than deploying contract with salt. i.e. I don't know what the alternative case is. Should/can we have another test with the unhappy path? Is it even possible to deploy without salt? This would help elucidate the full picture for a layman looking at the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MitchTurner thanks for the feedback. I will give more context on the test and then we can agree what to do.
We use the same contract code and have 3 Deploy "commands". First deploy will deploy the contract without random salt. The second deploy will do nothing as we already deploy the contract without random salt - the contract_id will be the same. And third deploy will use random salt so we will get a new contract_id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: show that two identical contracts can be deployed because their salt is different
3 participants