Skip to content

Implement prevrandao block_randomness in pallet-evm#246

Open
snowmead wants to merge 3 commits intomoonbeam-polkadot-stable2409from
snowmead/prevrandao-randomness
Open

Implement prevrandao block_randomness in pallet-evm#246
snowmead wants to merge 3 commits intomoonbeam-polkadot-stable2409from
snowmead/prevrandao-randomness

Conversation

@snowmead
Copy link

Get random in block_randomness from RanomnessProvider implementation passed in as a config to pallet-evm .

This randomness is not necessarily secure since it is generated and returned on the fly.
It is still recommended to use the BABE and local VRF precompiles to request randomness which is returned at a later time.

get random in `block_randomness` for prevrandao

#[pallet::no_default]
type RandomnessProvider: frame_support::traits::Randomness<
sp_core::H256,
Copy link

Choose a reason for hiding this comment

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

The runtime implementing this pallet may not what to enabled randomness. (For example, the template runtime which currently has a really dangerous example)

Suggested change
sp_core::H256,
Option<sp_core::H256>,

We can also put this type behind a feature, or just allow the random result to be None.

Copy link
Author

@snowmead snowmead Apr 17, 2025

Choose a reason for hiding this comment

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

I want to avoid changing this type to Option because I wouldn't be able to pass the randomness pallet directly as an impl since it implements <Hash, ...>. I would have to wrap it.

/// `AccountCodes` key size. 16 (hash) + 20 (key)
pub const ACCOUNT_CODES_KEY_SIZE: u64 = 36;
/// System block number.
pub const SYSTEM_BLOCK_NUMBER_PROOF_SIZE: u64 = 32;
Copy link

Choose a reason for hiding this comment

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

Not being used.

Comment on lines +340 to +357
pub struct RandomnessProvider;
impl
frame_support::traits::Randomness<
<Runtime as frame_system::Config>::Hash,
BlockNumberFor<Runtime>,
> for RandomnessProvider
{
fn random(
subject: &[u8],
) -> (
<Runtime as frame_system::Config>::Hash,
BlockNumberFor<Runtime>,
) {
let output = <Runtime as frame_system::Config>::Hashing::hash(subject);
let block_number = frame_system::Pallet::<Runtime>::block_number();
(output, block_number)
}
}
Copy link

Choose a reason for hiding this comment

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

This example is dangerous and we should change it. Suggestion in a comment above.

Copy link
Author

Choose a reason for hiding this comment

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

I could return a zero hash instead.

Copy link

Choose a reason for hiding this comment

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

Ignore my comment, this should be fine. The spec explicit says that this should not be used as a true randomness source.

Copy link

Choose a reason for hiding this comment

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

Sorry for coming back to this, could we append pallet_timestamp::Pallet::<Runtime>::now() to the subject.

And also add a note saying This is just an example, this should not be used as a true randomness source

@RomarQ
Copy link

RomarQ commented Apr 17, 2025

It is probably fine to add it to frontier, but we should not need it on moonbeam since there is no need. Similarly to arbitrum, they always return a constant value https://docs.arbitrum.io/build-decentralized-apps/arbitrum-vs-ethereum/solidity-support#differences-from-solidity-on-ethereum

@snowmead
Copy link
Author

It is probably fine to add it to frontier, but we should not need it on moonbeam since there is no need. Similarly to arbitrum, they always return a constant value https://docs.arbitrum.io/build-decentralized-apps/arbitrum-vs-ethereum/solidity-support#differences-from-solidity-on-ethereum

I think we should add it for the purposes of giving smart contract developers the ability to access randomness easily without going through the regular request/response scheme we have for BABE and local VRF.

We just need to ensure that they know this source of randomness is not to be trusted for critical operations. They could use it for games and other things that do not require much safety guarantee.

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.

2 participants