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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
- Allows to use `Result<Self, Error>` as a return type in constructors - [#1446](https://github.com/paritytech/ink/pull/1446)

- 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.


## Version 4.0.0-alpha.3

### Breaking Changes
Expand Down
4 changes: 2 additions & 2 deletions crates/e2e/macro/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ impl TryFrom<ast::AttributeArgs> for E2EConfig {
}
}
let additional_contracts = additional_contracts
.map(|(value, _)| value.value().split(" ").map(String::from).collect())
.unwrap_or_else(|| Vec::new());
.map(|(value, _)| value.value().split(' ').map(String::from).collect())
.unwrap_or_else(Vec::new);
Ok(E2EConfig {
ws_url: ws_url.map(|(value, _)| value),
additional_contracts,
Expand Down
2 changes: 0 additions & 2 deletions crates/engine/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ sha2 = { version = "0.10" }
sha3 = { version = "0.10" }
blake2 = { version = "0.10" }

rand = { version = "0.8" }

# ECDSA for the off-chain environment.
secp256k1 = { version = "0.24", features = ["recovery", "global-context"], optional = true }

Expand Down
24 changes: 2 additions & 22 deletions crates/engine/src/exec_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ use super::types::{
Balance,
BlockNumber,
BlockTimestamp,
Hash,
};
use rand::Rng;

/// The context of a contract execution.
#[cfg_attr(test, derive(Debug, PartialEq, Eq))]
#[derive(Default)]
pub struct ExecContext {
/// The caller of the contract execution. Might be user or another contract.
///
Expand All @@ -44,23 +43,6 @@ pub struct ExecContext {
pub block_number: BlockNumber,
/// The current block timestamp.
pub block_timestamp: BlockTimestamp,
/// The randomization entropy for a block.
pub entropy: Hash,
}

impl Default for ExecContext {
fn default() -> Self {
let mut entropy: [u8; 32] = Default::default();
rand::thread_rng().fill(entropy.as_mut());
Self {
caller: None,
callee: None,
value_transferred: 0,
block_number: 0,
block_timestamp: 0,
entropy,
}
}
}

impl ExecContext {
Expand Down Expand Up @@ -101,10 +83,8 @@ mod tests {
assert_eq!(exec_cont.callee(), vec![13]);

exec_cont.reset();
exec_cont.entropy = Default::default();

let mut new_exec_cont = ExecContext::new();
new_exec_cont.entropy = Default::default();
let new_exec_cont = ExecContext::new();
assert_eq!(exec_cont, new_exec_cont);
}
}
32 changes: 0 additions & 32 deletions crates/engine/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ use crate::{
BlockTimestamp,
},
};
use rand::{
Rng,
SeedableRng,
};
use scale::Encode;
use std::panic::panic_any;

Expand Down Expand Up @@ -431,34 +427,6 @@ impl Engine {
set_output(output, &fee[..])
}

/// Returns a randomized hash.
///
/// # Note
///
/// - This is the off-chain environment implementation of `random`.
/// It provides the same behavior in that it will likely yield the
/// same hash for the same subjects within the same block (or
/// execution context).
///
/// # Example
///
/// ```rust
/// let engine = ink_engine::ext::Engine::default();
/// let subject = [0u8; 32];
/// let mut output = [0u8; 32];
/// engine.random(&subject, &mut output.as_mut_slice());
/// ```
pub fn random(&self, subject: &[u8], output: &mut &mut [u8]) {
let seed = (self.exec_context.entropy, subject).encode();
let mut digest = [0u8; 32];
Engine::hash_blake2_256(&seed, &mut digest);

let mut rng = rand::rngs::StdRng::from_seed(digest);
let mut rng_bytes: [u8; 32] = Default::default();
rng.fill(&mut rng_bytes);
set_output(output, &rng_bytes[..])
}

/// Calls the chain extension method registered at `func_id` with `input`.
pub fn call_chain_extension(
&mut self,
Expand Down
3 changes: 0 additions & 3 deletions crates/engine/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@

use derive_more::From;

/// Same type as the `DefaultEnvironment::Hash` type.
pub type Hash = [u8; 32];

/// Same type as the `DefaultEnvironment::BlockNumber` type.
pub type BlockNumber = u32;

Expand Down
3 changes: 0 additions & 3 deletions crates/env/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ secp256k1 = { version = "0.24", features = ["recovery", "global-context"], optio
#
# Sadly couldn't be marked as dev-dependency.
# Never use this crate outside the off-chain environment!
rand = { version = "0.8", default-features = false, features = ["alloc"], optional = true }
scale-info = { version = "2", default-features = false, features = ["derive"], optional = true }

[features]
Expand All @@ -62,8 +61,6 @@ std = [
"scale/std",
"scale-info/std",
"secp256k1",
"rand/std",
"rand/std_rng",
"num-traits/std",
# Enables hashing crates for off-chain environment.
"sha2",
Expand Down
28 changes: 0 additions & 28 deletions crates/env/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,34 +405,6 @@ where
})
}

/// Returns a random hash seed and the block number since which it was determinable
/// by chain observers.
///
/// # Note
///
/// - The subject buffer can be used to further randomize the hash.
/// - Within the same execution returns the same random hash for the same subject.
///
/// # Errors
///
/// If the returned value cannot be properly decoded.
///
/// # Important
///
/// The returned seed should only be used to distinguish commitments made before
/// the returned block number. If the block number is too early (i.e. commitments were
/// made afterwards), then ensure no further commitments may be made and repeatedly
/// call this on later blocks until the block number returned is later than the latest
/// commitment.
pub fn random<E>(subject: &[u8]) -> Result<(E::Hash, E::BlockNumber)>
where
E: Environment,
{
<EnvInstance as OnInstance>::on_instance(|instance| {
TypedEnvBackend::random::<E>(instance, subject)
})
}

/// Appends the given message to the debug message buffer.
pub fn debug_message(message: &str) {
<EnvInstance as OnInstance>::on_instance(|instance| {
Expand Down
9 changes: 0 additions & 9 deletions crates/env/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,6 @@ pub trait TypedEnvBackend: EnvBackend {
where
E: Environment;

/// Returns a random hash seed.
///
/// # Note
///
/// For more details visit: [`random`][`crate::random`]
fn random<E>(&mut self, subject: &[u8]) -> Result<(E::Hash, E::BlockNumber)>
where
E: Environment;

/// Checks whether a specified account belongs to a contract.
///
/// # Note
Expand Down
9 changes: 0 additions & 9 deletions crates/env/src/engine/off_chain/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,6 @@ impl TypedEnvBackend for EnvInstance {
})
}

fn random<E>(&mut self, subject: &[u8]) -> Result<(E::Hash, E::BlockNumber)>
where
E: Environment,
{
let mut output: [u8; BUFFER_SIZE] = [0; BUFFER_SIZE];
self.engine.random(subject, &mut &mut output[..]);
scale::Decode::decode(&mut &output[..]).map_err(Into::into)
}

fn is_contract<E>(&mut self, _account: &E::AccountId) -> bool
where
E: Environment,
Expand Down
12 changes: 0 additions & 12 deletions crates/env/src/engine/off_chain/test_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,6 @@ where
})
}

/// Set the entropy hash of the current block.
///
/// # Note
///
/// This allows to control what [`random`][`crate::random`] returns.
pub fn set_block_entropy<T>(_entropy: T::Hash) -> Result<()>
where
T: Environment,
{
unimplemented!("off-chain environment does not yet support `set_block_entropy`");
}

/// Returns the contents of the past performed environmental debug messages in order.
pub fn recorded_debug_messages() -> RecordedDebugMessages {
<EnvInstance as OnInstance>::on_instance(|instance| {
Expand Down
8 changes: 0 additions & 8 deletions crates/env/src/engine/on_chain/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,6 @@ impl<'a> ScopedBuffer<'a> {
lhs
}

/// Returns a buffer scope filled with `bytes` with the proper length.
pub fn take_bytes(&mut self, bytes: &[u8]) -> &'a mut [u8] {
debug_assert_eq!(self.offset, 0);
let buffer = self.take(bytes.len());
buffer.copy_from_slice(bytes);
buffer
}

/// Encode the given value into the scoped buffer and return the sub slice
/// containing all the encoded bytes.
#[inline(always)]
Expand Down
23 changes: 0 additions & 23 deletions crates/env/src/engine/on_chain/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,6 @@ mod sys {

pub fn seal_terminate(beneficiary_ptr: Ptr32<[u8]>) -> !;

pub fn seal_random(
subject_ptr: Ptr32<[u8]>,
subject_len: u32,
output_ptr: Ptr32Mut<[u8]>,
output_len_ptr: Ptr32Mut<u32>,
);

pub fn seal_call(
flags: u32,
callee_ptr: Ptr32<[u8]>,
Expand Down Expand Up @@ -675,22 +668,6 @@ pub fn weight_to_fee(gas: u64, output: &mut &mut [u8]) {
extract_from_slice(output, output_len as usize);
}

#[inline(always)]
pub fn random(subject: &[u8], output: &mut &mut [u8]) {
let mut output_len = output.len() as u32;
{
unsafe {
sys::seal_random(
Ptr32::from_slice(subject),
subject.len() as u32,
Ptr32Mut::from_slice(output),
Ptr32Mut::from_ref(&mut output_len),
)
};
}
extract_from_slice(output, output_len as usize);
}

#[cfg(feature = "ink-debug")]
/// Call `seal_debug_message` with the supplied UTF-8 encoded message.
///
Expand Down
11 changes: 0 additions & 11 deletions crates/env/src/engine/on_chain/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,17 +523,6 @@ impl TypedEnvBackend for EnvInstance {
<E::Balance as FromLittleEndian>::from_le_bytes(result)
}

fn random<E>(&mut self, subject: &[u8]) -> Result<(E::Hash, E::BlockNumber)>
where
E: Environment,
{
let mut scope = self.scoped_buffer();
let enc_subject = scope.take_bytes(subject);
let output = &mut scope.take_rest();
ext::random(enc_subject, output);
scale::Decode::decode(&mut &output[..]).map_err(Into::into)
}

fn is_contract<E>(&mut self, account_id: &E::AccountId) -> bool
where
E: Environment,
Expand Down
34 changes: 0 additions & 34 deletions crates/ink/src/env_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,40 +666,6 @@ where
ink_env::transfer::<E>(destination, value)
}

/// Returns a random hash seed.
///
/// # Example
///
/// ```
/// # #[ink::contract]
/// # pub mod my_contract {
/// # #[ink(storage)]
/// # pub struct MyContract { }
/// #
/// # impl MyContract {
/// # #[ink(constructor)]
/// # pub fn new() -> Self {
/// # Self {}
/// # }
/// #
/// #[ink(message)]
/// pub fn random_bool(&self) -> bool {
/// let additional_randomness = b"seed";
/// let (hash, _block_number) = self.env().random(additional_randomness);
/// hash.as_ref()[0] != 0
/// }
/// #
/// # }
/// # }
/// ```
///
/// # Note
///
/// For more details visit: [`ink_env::random`]
pub fn random(self, subject: &[u8]) -> (E::Hash, E::BlockNumber) {
ink_env::random::<E>(subject).expect("couldn't decode randomized hash")
}

/// Computes the hash of the given bytes using the cryptographic hash `H`.
///
/// # Example
Expand Down
2 changes: 0 additions & 2 deletions crates/ink/tests/ui/contract/pass/env-access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ mod contract {
let _ = Self::env().caller();
let _ = Self::env().gas_left();
let _ = Self::env().minimum_balance();
let _ = Self::env().random(&[]);
let _ = Self::env().transferred_value();
let _ = Self::env().weight_to_fee(0);
Self {}
Expand All @@ -28,7 +27,6 @@ mod contract {
let _ = self.env().caller();
let _ = self.env().gas_left();
let _ = self.env().minimum_balance();
let _ = self.env().random(&[]);
let _ = self.env().transferred_value();
let _ = self.env().weight_to_fee(0);
}
Expand Down