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

Remove random function #1442

Merged
merged 3 commits into from
Oct 29, 2022
Merged

Remove random function #1442

merged 3 commits into from
Oct 29, 2022

Conversation

athei
Copy link
Contributor

@athei athei commented Oct 19, 2022

We need to move away from on-chain randomness. We cannot rely on hosting chains to provide unpredictable randomness. Like on ethereum that should be provided by oracles or maybe a chain extension of a chain has a way to provide randomness. It should not be part of the core API.

@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Remove random function from ink!.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Remove random function from ink!.
### Removed
- Remove random function ‒ [#1442](https://github.com/paritytech/ink/pull/1442)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add some more information in a section ### How to replace random here?

My concern is that we're just removing it without providing guidance on how to substitute this functionality.

Is there no pallet out there which we could create a chain extension with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On ethereum contracts need to look for randomness on their own anyways. They rely on oracle contracts AFAIK. If we had some reliable way to provide randomness in the general case we wouldn't remove it. I don't think there is anything at the moment.

@burdges has some ideas but nothing readily available. So we can't really give any advice beyond "good luck bro".

Copy link
Contributor

Choose a reason for hiding this comment

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

This pseudo random function was useful enough for our contract but now its removed. Any guidance on how to recreate this functionality would be appreciated.

Copy link
Contributor

@forgetso forgetso Jan 12, 2023

Choose a reason for hiding this comment

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

We need a random seed per block that is good enough to randomly read a value from a vector within our contract. There is no transaction involved. The operation is not high value and the seed calculated by the validators was fulfilling this purpose very well. Are the validators no longer committing this random seed to each block?

Could the current block hash be made available in ink! as a substitute?

Copy link

@burdges burdges Jan 12, 2023

Choose a reason for hiding this comment

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

There are several randomness sources available in polkadot, but they won't all be available in all ink deployments, especially ink deployments on stand along chains. We'd really like deployers to understand their limitations.

A block hash based randomness is good when you're doing a Fiat-Shamir transform, but usually not so great otherwise, but if your usage is low enough value then fine. There is nothing wrong with a parachain block knowing its own block hash, because although this makes parachain execution non-deterministic relative to when the block is built, it clearly stays deterministic from the perspective of polkadot. It's fine to make the block hash available and just warn people that they should look at other randomness options if they think about using it for randomness.

BABE's VRF usage provides three slightly different randomness sources, each with their own use cases. Sassafras should add a fourth, but not too different either. ParentBlockRandomness might fit your use case.

A drand gives strong fresh randomness every 15 seconds. It's desynced from polkadot though, so one needs a "drand bridge" to sync reads from drand, without giving someone too much authority to tweak the syncing.

Advanced randomness like polkadot's candle auctions require some sort of commitment before the randomness exists, after which the chain chooses the randomness with which the commitment gets used. VRF keys held by users fit roughly into this category too, but less strictly.

Copy link
Contributor Author

@athei athei Jan 15, 2023

Choose a reason for hiding this comment

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

We need a random seed per block that is good enough to randomly read a value from a vector within our contract. There is no transaction involved. The operation is not high value and the seed calculated by the validators was fulfilling this purpose very well. Are the validators no longer committing this random seed to each block?

Could the current block hash be made available in ink! as a substitute?

We would need way for information to give some advice and to determine if using predictable randomness is actually safe. I recommend asking on stack exchange.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've provided more info on stack exchange.

@codecov-commenter
Copy link

codecov-commenter commented Oct 29, 2022

Codecov Report

Merging #1442 (c5329c2) into master (6360098) will decrease coverage by 5.96%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1442      +/-   ##
==========================================
- Coverage   70.39%   64.43%   -5.97%     
==========================================
  Files         200      200              
  Lines        6137     6101      -36     
==========================================
- Hits         4320     3931     -389     
- Misses       1817     2170     +353     
Impacted Files Coverage Δ
crates/e2e/macro/src/config.rs 56.75% <0.00%> (-1.14%) ⬇️
crates/engine/src/exec_context.rs 100.00% <ø> (ø)
crates/engine/src/ext.rs 67.87% <ø> (-2.65%) ⬇️
crates/engine/src/types.rs 66.66% <ø> (ø)
crates/env/src/api.rs 33.87% <ø> (+1.56%) ⬆️
crates/env/src/backend.rs 83.33% <ø> (ø)
crates/env/src/engine/off_chain/impls.rs 45.67% <ø> (+1.10%) ⬆️
crates/env/src/engine/off_chain/test_api.rs 64.70% <ø> (ø)
crates/ink/src/env_access.rs 13.04% <ø> (+1.04%) ⬆️
crates/ink/tests/ui/contract/pass/env-access.rs 4.34% <ø> (+0.34%) ⬆️
... and 27 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ascjones ascjones merged commit 6f9e4c8 into master Oct 29, 2022
@ascjones ascjones deleted the at/remove-random branch October 29, 2022 13:28
@ascjones ascjones mentioned this pull request Nov 22, 2022
1 task
@ascjones ascjones mentioned this pull request Feb 15, 2023
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.

6 participants