Skip to content

remove the random builtin function#1041

Merged
xermicus merged 1 commit intohyperledger-solang:mainfrom
xermicus:remove_random_builtin
Oct 19, 2022
Merged

remove the random builtin function#1041
xermicus merged 1 commit intohyperledger-solang:mainfrom
xermicus:remove_random_builtin

Conversation

@xermicus
Copy link
Contributor

@xermicus xermicus commented Oct 19, 2022 •
edited
Loading

It is unsafe for most use-cases and will be deprecated in ink!. Contract authors should rather use VRF from chain extensions or oracles.

The seal_random API is still used to calculate the salt on instantiation (if it is missing). There will be a new API seal_nonce which can be used to replace this. I'll do this in another PR once the new nonce API is available in pallets contract.

Side note: In my opinion it is completely safe to remove this built in. It was completely broken for IDK exactly how long but no one complained.

@xermicus
remove random builtin
3635e84 
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
@xermicus xermicus requested a review from LucasSte October 19, 2022 09:59
@xermicus xermicus requested a review from seanyoung as a code owner October 19, 2022 09:59
@athei
Copy link

athei commented Oct 19, 2022 •
edited
Loading

Thanks. Looks good to me. I don't see seal_random being removed from the backend, though.

@seanyoung
Copy link
Contributor

seanyoung commented Oct 19, 2022 •
edited
Loading

Thanks. Looks good to me. I don't see seal_random being removed from the backend, though.

Would it make sense to keep this function in solang until VRF is usable from solang?

Also out of curiosity, what was broken with the implementation of seal_random()? I thought it was very nice.

@xermicus
Copy link
Contributor Author

xermicus commented Oct 19, 2022 •
edited
Loading

Would it make sense to keep this function in solang until VRF is usable from solang?

We still need it during contract instantiation, if the salt is missing. I'll remove it completely as soon as the nonce API is available. As for now I don't think we should be too concerned about providing VRF to the users directly via solang... I think it is 1. too specific depending on the target runtime env and 2. it can be done in user space / via oracles or chain extensions.

Also out of curiosity, what was broken with the implementation of seal_random()? I thought it was very nice.

It is potentially predictable.

@LucasSte
Copy link
Contributor

LucasSte commented Oct 19, 2022

remove random builtin

This title seems like you have randomly chosen a builtin in Solang and removed it.

@xermicus xermicus changed the title remove random builtin remove the random builtin function Oct 19, 2022
LucasSte
LucasSte approved these changes Oct 19, 2022
View reviewed changes
Copy link
Contributor

@LucasSte LucasSte left a comment

Choose a reason for hiding this comment

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

Please change your PR's title to something less ambiguous.
A suggestion would be Remove Substrate 'random' builtin.

@xermicus xermicus merged commit cb3b8b0 into hyperledger-solang:main Oct 19, 2022
xermicus added a commit to xermicus/solang that referenced this pull request Nov 7, 2022
@xermicus
remove builtin random function (hyperledger-solang#1041)
d5b36a5 
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
@athei athei mentioned this pull request Jan 22, 2023
Merged
@xermicus xermicus deleted the remove_random_builtin branch May 15, 2023 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanyoung seanyoung seanyoung approved these changes

@LucasSte LucasSte LucasSte approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xermicus @athei @seanyoung @LucasSte