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

salt is of type bytes32 #1368

Merged
merged 1 commit into from
Jun 20, 2023
Merged

salt is of type bytes32 #1368

merged 1 commit into from
Jun 20, 2023

Conversation

seanyoung
Copy link
Contributor

@seanyoung seanyoung commented Jun 16, 2023

solang mistakenly uses uint256 as the type for the salt argument. Change this to bytes32 to be compatible with solc.

Comment on lines 82 to 83
either a random value or it can be explicitly set using the
``{salt: hex"439d399ee3b5b0fae6c8d567a8cbfa22d59f8f2c5fe308fd0a92366c116e5f1a"}`` syntax. A
Copy link
Contributor

@LucasSte LucasSte Jun 16, 2023

Choose a reason for hiding this comment

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

When you use either ... or ..., the grammatical forms before and after the or must be the same. a random value is a noun phrase and it can be explicitly set is a verb clause.

I suggest writing The salt can either be a random value or set explicitly using the syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've re-written this paragraph

syntax, or if it is omitted, then a random value is used.
Specifying a salt will remove the need for generating a random value at runtime, however
care must be taken to avoid using the same salt more than once. Creating a contract twice
with the same salt and arguments will fail. The salt is of type ``bytes32``.
Copy link
Contributor

@LucasSte LucasSte Jun 19, 2023

Choose a reason for hiding this comment

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

This paragraph as it stands is very messy. I suggest changing the first part to this setting:

When a new contract is created, its address will be a hash of the constructor arguments (the contract's input). As the salt is concatenated to the input, a contract cannot be created twice with the same arguments. The salt can be set using the {salt: hex"439d399ee3b5b0fae6c8d567a8cbfa22d59f8f2c5fe308fd0a92366c116e5f1a"} syntax or assume a random value if none is given.

The end of it can remain with your wording (no changes here):

Specifying a salt will remove the need for generating a random value at runtime, however
care must be taken to avoid using the same salt more than once. Creating a contract twice
with the same salt and arguments will fail. The salt is of type bytes32.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The salt isn't concatenated to the input, the input and the salt are hashed. I've re-phrased that to make that clearer.

It's much better now, thanks

Signed-off-by: Sean Young <[email protected]>
@seanyoung seanyoung merged commit 0221f20 into hyperledger-solang:main Jun 20, 2023
@seanyoung seanyoung deleted the salt branch June 20, 2023 14:56
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.

3 participants