From 89664442760fd358dd2743ecfebcb8d9e9f18182 Mon Sep 17 00:00:00 2001 From: Amin Moghaddam Date: Thu, 15 Aug 2024 11:46:42 +0700 Subject: [PATCH] feat(entropy): Limit number of hashes (#1822) * Add revert reasons to Entropy tests * Revert if numHashes > maxNumHashes * Add update commitment function * Set maximum number of hashes in contract + keeper --- apps/fortuna/Cargo.lock | 2 +- apps/fortuna/Cargo.toml | 2 +- apps/fortuna/src/chain/ethereum.rs | 14 +- apps/fortuna/src/command/setup_provider.rs | 28 ++ apps/fortuna/src/config.rs | 4 + apps/fortuna/src/keeper.rs | 122 ++++++--- .../contracts/contracts/entropy/Entropy.sol | 70 +++++ .../contracts/entropy/EntropyUpgradable.sol | 2 +- .../contracts/forge-test/Entropy.t.sol | 240 +++++++++++++++--- .../entropy_sdk/solidity/EntropyErrors.sol | 5 + .../entropy_sdk/solidity/EntropyEvents.sol | 5 + .../entropy_sdk/solidity/EntropyStructs.sol | 3 + .../entropy_sdk/solidity/IEntropy.sol | 12 + .../solidity/abis/EntropyErrors.json | 10 + .../solidity/abis/EntropyEvents.json | 30 +++ .../entropy_sdk/solidity/abis/IEntropy.json | 71 ++++++ 16 files changed, 547 insertions(+), 73 deletions(-) diff --git a/apps/fortuna/Cargo.lock b/apps/fortuna/Cargo.lock index 39e2d7bc05..b70aa79dc8 100644 --- a/apps/fortuna/Cargo.lock +++ b/apps/fortuna/Cargo.lock @@ -1502,7 +1502,7 @@ dependencies = [ [[package]] name = "fortuna" -version = "6.4.2" +version = "6.5.2" dependencies = [ "anyhow", "axum", diff --git a/apps/fortuna/Cargo.toml b/apps/fortuna/Cargo.toml index 86dcea0d66..0ee4efa670 100644 --- a/apps/fortuna/Cargo.toml +++ b/apps/fortuna/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fortuna" -version = "6.4.2" +version = "6.5.2" edition = "2021" [dependencies] diff --git a/apps/fortuna/src/chain/ethereum.rs b/apps/fortuna/src/chain/ethereum.rs index 52a6b94d8d..abf371b589 100644 --- a/apps/fortuna/src/chain/ethereum.rs +++ b/apps/fortuna/src/chain/ethereum.rs @@ -28,6 +28,7 @@ use { abi::RawLog, contract::{ abigen, + ContractCall, EthLogDecode, }, core::types::Address, @@ -72,17 +73,18 @@ abigen!( "../../target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json" ); -pub type SignablePythContractInner = PythRandom< - LegacyTxMiddleware< - GasOracleMiddleware< - NonceManagerMiddleware, LocalWallet>>, - EthProviderOracle>, - >, +pub type MiddlewaresWrapper = LegacyTxMiddleware< + GasOracleMiddleware< + NonceManagerMiddleware, LocalWallet>>, + EthProviderOracle>, >, >; +pub type SignablePythContractInner = PythRandom>; pub type SignablePythContract = SignablePythContractInner; pub type InstrumentedSignablePythContract = SignablePythContractInner; +pub type PythContractCall = ContractCall, ()>; + pub type PythContract = PythRandom>; pub type InstrumentedPythContract = PythRandom>; diff --git a/apps/fortuna/src/command/setup_provider.rs b/apps/fortuna/src/command/setup_provider.rs index a8d31a5860..740323c807 100644 --- a/apps/fortuna/src/command/setup_provider.rs +++ b/apps/fortuna/src/command/setup_provider.rs @@ -193,6 +193,14 @@ async fn setup_chain_provider( .in_current_span() .await?; + sync_max_num_hashes( + &contract, + &provider_info, + chain_config.max_num_hashes.unwrap_or(0), + ) + .in_current_span() + .await?; + Ok(()) } @@ -248,3 +256,23 @@ async fn sync_fee_manager( } Ok(()) } + + +async fn sync_max_num_hashes( + contract: &Arc, + provider_info: &ProviderInfo, + max_num_hashes: u32, +) -> Result<()> { + if provider_info.max_num_hashes != max_num_hashes { + tracing::info!("Updating provider max num hashes to {:?}", max_num_hashes); + if let Some(receipt) = contract + .set_max_num_hashes(max_num_hashes) + .send() + .await? + .await? + { + tracing::info!("Updated provider max num hashes to : {:?}", receipt); + } + } + Ok(()) +} diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs index d7bf642407..15a048d442 100644 --- a/apps/fortuna/src/config.rs +++ b/apps/fortuna/src/config.rs @@ -183,6 +183,10 @@ pub struct EthereumConfig { /// Historical commitments made by the provider. pub commitments: Option>, + + /// Maximum number of hashes to record in a request. + /// This should be set according to the maximum gas limit the provider supports for callbacks. + pub max_num_hashes: Option, } diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index 8cca89d9e9..49be70a9e3 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -10,6 +10,7 @@ use { ethereum::{ InstrumentedPythContract, InstrumentedSignablePythContract, + PythContractCall, }, reader::{ BlockNumber, @@ -87,6 +88,10 @@ const TRACK_INTERVAL: Duration = Duration::from_secs(10); const WITHDRAW_INTERVAL: Duration = Duration::from_secs(300); /// Check whether we need to adjust the fee at this interval. const ADJUST_FEE_INTERVAL: Duration = Duration::from_secs(30); +/// Check whether we need to manually update the commitments to reduce numHashes for future +/// requests and reduce the gas cost of the reveal. +const UPDATE_COMMITMENTS_INTERVAL: Duration = Duration::from_secs(30); +const UPDATE_COMMITMENTS_THRESHOLD_FACTOR: f64 = 0.95; /// Rety last N blocks const RETRY_PREVIOUS_BLOCKS: u64 = 100; @@ -314,6 +319,8 @@ pub async fn run_keeper_threads( .in_current_span(), ); + spawn(update_commitments_loop(contract.clone(), chain_state.clone()).in_current_span()); + // Spawn a thread to track the provider info and the balance of the keeper spawn( @@ -960,21 +967,7 @@ pub async fn withdraw_fees_if_necessary( if keeper_balance < min_balance && U256::from(fees) > min_balance { tracing::info!("Claiming accrued fees..."); let contract_call = contract.withdraw_as_fee_manager(provider_address, fees); - let pending_tx = contract_call - .send() - .await - .map_err(|e| anyhow!("Error submitting the withdrawal transaction: {:?}", e))?; - - let tx_result = pending_tx - .await - .map_err(|e| anyhow!("Error waiting for withdrawal transaction receipt: {:?}", e))? - .ok_or_else(|| anyhow!("Can't verify the withdrawal, probably dropped from mempool"))?; - - tracing::info!( - transaction_hash = &tx_result.transaction_hash.to_string(), - "Withdrew fees to keeper address. Receipt: {:?}", - tx_result, - ); + send_and_confirm(contract_call).await?; } else if keeper_balance < min_balance { tracing::warn!("Keeper balance {:?} is too low (< {:?}) but provider fees are not sufficient to top-up.", keeper_balance, min_balance) } @@ -982,6 +975,38 @@ pub async fn withdraw_fees_if_necessary( Ok(()) } +pub async fn send_and_confirm(contract_call: PythContractCall) -> Result<()> { + let call_name = contract_call.function.name.as_str(); + let pending_tx = contract_call + .send() + .await + .map_err(|e| anyhow!("Error submitting transaction({}) {:?}", call_name, e))?; + + let tx_result = pending_tx + .await + .map_err(|e| { + anyhow!( + "Error waiting for transaction({}) receipt: {:?}", + call_name, + e + ) + })? + .ok_or_else(|| { + anyhow!( + "Can't verify the transaction({}), probably dropped from mempool", + call_name + ) + })?; + + tracing::info!( + transaction_hash = &tx_result.transaction_hash.to_string(), + "Confirmed transaction({}). Receipt: {:?}", + call_name, + tx_result, + ); + Ok(()) +} + #[tracing::instrument(name = "adjust_fee", skip_all)] pub async fn adjust_fee_wrapper( contract: Arc, @@ -1020,6 +1045,55 @@ pub async fn adjust_fee_wrapper( } } +#[tracing::instrument(name = "update_commitments", skip_all)] +pub async fn update_commitments_loop( + contract: Arc, + chain_state: BlockchainState, +) { + loop { + if let Err(e) = update_commitments_if_necessary(contract.clone(), &chain_state) + .in_current_span() + .await + { + tracing::error!("Update commitments. error: {:?}", e); + } + time::sleep(UPDATE_COMMITMENTS_INTERVAL).await; + } +} + + +pub async fn update_commitments_if_necessary( + contract: Arc, + chain_state: &BlockchainState, +) -> Result<()> { + //TODO: we can reuse the result from the last call from the watch_blocks thread to reduce RPCs + let latest_safe_block = get_latest_safe_block(&chain_state).in_current_span().await; + let provider_address = chain_state.provider_address; + let provider_info = contract + .get_provider_info(provider_address) + .block(latest_safe_block) // To ensure we are not revealing sooner than we should + .call() + .await + .map_err(|e| anyhow!("Error while getting provider info. error: {:?}", e))?; + if provider_info.max_num_hashes == 0 { + return Ok(()); + } + let threshold = + ((provider_info.max_num_hashes as f64) * UPDATE_COMMITMENTS_THRESHOLD_FACTOR) as u64; + if provider_info.sequence_number - provider_info.current_commitment_sequence_number > threshold + { + let seq_number = provider_info.sequence_number - 1; + let provider_revelation = chain_state + .state + .reveal(seq_number) + .map_err(|e| anyhow!("Error revealing: {:?}", e))?; + let contract_call = + contract.advance_provider_commitment(provider_address, seq_number, provider_revelation); + send_and_confirm(contract_call).await?; + } + Ok(()) +} + /// Adjust the fee charged by the provider to ensure that it is profitable at the prevailing gas price. /// This method targets a fee as a function of the maximum cost of the callback, /// c = (gas_limit) * (current gas price), with min_fee_wei as a lower bound on the fee. @@ -1105,23 +1179,7 @@ pub async fn adjust_fee_if_necessary( target_fee ); let contract_call = contract.set_provider_fee_as_fee_manager(provider_address, target_fee); - let pending_tx = contract_call - .send() - .await - .map_err(|e| anyhow!("Error submitting the set fee transaction: {:?}", e))?; - - let tx_result = pending_tx - .await - .map_err(|e| anyhow!("Error waiting for set fee transaction receipt: {:?}", e))? - .ok_or_else(|| { - anyhow!("Can't verify the set fee transaction, probably dropped from mempool") - })?; - - tracing::info!( - transaction_hash = &tx_result.transaction_hash.to_string(), - "Set provider fee. Receipt: {:?}", - tx_result, - ); + send_and_confirm(contract_call).await?; *sequence_number_of_last_fee_update = Some(provider_info.sequence_number); } else { diff --git a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol index 4d59882d11..ede2e39976 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol @@ -234,6 +234,12 @@ abstract contract Entropy is IEntropy, EntropyState { assignedSequenceNumber - providerInfo.currentCommitmentSequenceNumber ); + if ( + providerInfo.maxNumHashes != 0 && + req.numHashes > providerInfo.maxNumHashes + ) { + revert EntropyErrors.LastRevealedTooOld(); + } req.commitment = keccak256( bytes.concat(userCommitment, providerInfo.currentCommitment) ); @@ -351,6 +357,51 @@ abstract contract Entropy is IEntropy, EntropyState { } } + // Advance the provider commitment and increase the sequence number. + // This is used to reduce the `numHashes` required for future requests which leads to reduced gas usage. + function advanceProviderCommitment( + address provider, + uint64 advancedSequenceNumber, + bytes32 providerRevelation + ) public override { + EntropyStructs.ProviderInfo storage providerInfo = _state.providers[ + provider + ]; + if ( + advancedSequenceNumber <= + providerInfo.currentCommitmentSequenceNumber + ) revert EntropyErrors.UpdateTooOld(); + if (advancedSequenceNumber >= providerInfo.endSequenceNumber) + revert EntropyErrors.AssertionFailure(); + + uint32 numHashes = SafeCast.toUint32( + advancedSequenceNumber - + providerInfo.currentCommitmentSequenceNumber + ); + bytes32 providerCommitment = constructProviderCommitment( + numHashes, + providerRevelation + ); + + if (providerCommitment != providerInfo.currentCommitment) + revert EntropyErrors.IncorrectRevelation(); + + providerInfo.currentCommitmentSequenceNumber = advancedSequenceNumber; + providerInfo.currentCommitment = providerRevelation; + if ( + providerInfo.currentCommitmentSequenceNumber >= + providerInfo.sequenceNumber + ) { + // This means the provider called the function with a sequence number that was not yet requested. + // Providers should never do this and we consider such an implementation flawed. + // Assuming this is landed on-chain it's better to bump the sequence number and never use that range + // for future requests. Otherwise, someone can use the leaked revelation to derive favorable random numbers. + providerInfo.sequenceNumber = + providerInfo.currentCommitmentSequenceNumber + + 1; + } + } + // Fulfill a request for a random number. This method validates the provided userRandomness and provider's proof // against the corresponding commitments in the in-flight request. If both values are validated, this function returns // the corresponding random number. @@ -555,6 +606,25 @@ abstract contract Entropy is IEntropy, EntropyState { emit ProviderFeeManagerUpdated(msg.sender, oldFeeManager, manager); } + // Set the maximum number of hashes to record in a request. This should be set according to the maximum gas limit + // the provider supports for callbacks. + function setMaxNumHashes(uint32 maxNumHashes) external override { + EntropyStructs.ProviderInfo storage provider = _state.providers[ + msg.sender + ]; + if (provider.sequenceNumber == 0) { + revert EntropyErrors.NoSuchProvider(); + } + + uint32 oldMaxNumHashes = provider.maxNumHashes; + provider.maxNumHashes = maxNumHashes; + emit ProviderMaxNumHashesAdvanced( + msg.sender, + oldMaxNumHashes, + maxNumHashes + ); + } + function constructUserCommitment( bytes32 userRandomness ) public pure override returns (bytes32 userCommitment) { diff --git a/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol b/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol index a7fae59b8f..a56cc90682 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/EntropyUpgradable.sol @@ -105,6 +105,6 @@ contract EntropyUpgradable is } function version() public pure returns (string memory) { - return "0.3.1"; + return "0.4.0"; } } diff --git a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol index 9cfed632b7..d088bb0c37 100644 --- a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol @@ -22,7 +22,8 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { address public provider1 = address(1); bytes32[] provider1Proofs; uint128 provider1FeeInWei = 8; - uint64 provider1ChainLength = 100; + uint64 provider1ChainLength = 1000; + uint32 provider1MaxNumHashes = 500; bytes provider1Uri = bytes("https://foo.com"); bytes provider1CommitmentMetadata = hex"0100"; @@ -65,6 +66,8 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { provider1ChainLength, provider1Uri ); + vm.prank(provider1); + random.setMaxNumHashes(provider1MaxNumHashes); bytes32[] memory hashChain2 = generateHashChain(provider2, 0, 100); provider2Proofs = hashChain2; @@ -115,25 +118,15 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { uint fee, address provider, uint randomNumber, - bool useBlockhash + bool useBlockhash, + bytes4 revertReason ) public { - // Note: for some reason vm.expectRevert() won't catch errors from the request function (?!), - // even though they definitely revert. Use a try/catch instead for the moment, though the try/catch - // doesn't let you simulate the msg.sender. However, it's fine if the msg.sender is the test contract. - bool requestSucceeds = false; - try - random.request{value: fee}( - provider, - random.constructUserCommitment(bytes32(uint256(randomNumber))), - useBlockhash - ) - { - requestSucceeds = true; - } catch { - requestSucceeds = false; - } - - assert(!requestSucceeds); + bytes32 userCommitment = random.constructUserCommitment( + bytes32(uint256(randomNumber)) + ); + vm.deal(address(this), fee); + vm.expectRevert(revertReason); + random.request{value: fee}(provider, userCommitment, useBlockhash); } function assertRevealSucceeds( @@ -275,7 +268,13 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { } function testNoSuchProvider() public { - assertRequestReverts(10000000, unregisteredProvider, 42, false); + assertRequestReverts( + 10000000, + unregisteredProvider, + 42, + false, + EntropyErrors.NoSuchProvider.selector + ); } function testAuthorization() public { @@ -571,14 +570,23 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { function testOutOfRandomness() public { // Should be able to request chainLength - 1 random numbers successfully. for (uint64 i = 0; i < provider1ChainLength - 1; i++) { - request(user1, provider1, i, false); + uint64 sequenceNumber = request(user2, provider1, 42, false); + assertRevealSucceeds( + user2, + provider1, + sequenceNumber, + 42, + provider1Proofs[sequenceNumber], + ALL_ZEROS + ); } assertRequestReverts( random.getFee(provider1), provider1, provider1ChainLength - 1, - false + false, + EntropyErrors.OutOfRandomness.selector ); } @@ -603,34 +611,54 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { } function testOverflow() public { + bytes32 userCommitment = random.constructUserCommitment( + bytes32(uint256(42)) + ); // msg.value overflows the uint128 fee variable - assertRequestReverts(2 ** 128, provider1, 42, false); + uint fee = 2 ** 128; + vm.deal(address(this), fee); + vm.expectRevert("SafeCast: value doesn't fit in 128 bits"); + random.request{value: fee}(provider1, userCommitment, false); // block number is too large vm.roll(2 ** 96); - assertRequestReverts( - pythFeeInWei + provider1FeeInWei, + vm.expectRevert("SafeCast: value doesn't fit in 64 bits"); + random.request{value: pythFeeInWei + provider1FeeInWei}( provider1, - 42, + userCommitment, true ); } function testFees() public { // Insufficient fees causes a revert - assertRequestReverts(0, provider1, 42, false); + assertRequestReverts( + 0, + provider1, + 42, + false, + EntropyErrors.InsufficientFee.selector + ); assertRequestReverts( pythFeeInWei + provider1FeeInWei - 1, provider1, 42, - false + false, + EntropyErrors.InsufficientFee.selector + ); + assertRequestReverts( + 0, + provider2, + 42, + false, + EntropyErrors.InsufficientFee.selector ); - assertRequestReverts(0, provider2, 42, false); assertRequestReverts( pythFeeInWei + provider2FeeInWei - 1, provider2, 42, - false + false, + EntropyErrors.InsufficientFee.selector ); // Accrue some fees for both providers @@ -669,7 +697,13 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { provider1Uri ); - assertRequestReverts(pythFeeInWei + 12345 - 1, provider1, 42, false); + assertRequestReverts( + pythFeeInWei + 12345 - 1, + provider1, + 42, + false, + EntropyErrors.InsufficientFee.selector + ); requestWithFee(user2, pythFeeInWei + 12345, provider1, 42, false); uint128 providerOneBalance = provider1FeeInWei * 3 + 12345; @@ -788,7 +822,6 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { random.getRequest(provider1, assignedSequenceNumber).provider, provider1 ); - vm.expectRevert(EntropyErrors.InvalidRevealCall.selector); random.reveal( provider1, @@ -927,6 +960,149 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { ); } + function testLastRevealedTooOld() public { + for (uint256 i = 0; i < provider1MaxNumHashes; i++) { + request(user1, provider1, 42, false); + } + assertRequestReverts( + random.getFee(provider1), + provider1, + 42, + false, + EntropyErrors.LastRevealedTooOld.selector + ); + } + + function testAdvanceProviderCommitment( + uint32 requestCount, + uint32 updateSeqNumber + ) public { + vm.assume(requestCount < provider1MaxNumHashes); + vm.assume(updateSeqNumber < requestCount); + vm.assume(0 < updateSeqNumber); + + for (uint256 i = 0; i < requestCount; i++) { + request(user1, provider1, 42, false); + } + assertInvariants(); + EntropyStructs.ProviderInfo memory info1 = random.getProviderInfo( + provider1 + ); + assertEq(info1.currentCommitmentSequenceNumber, 0); + assertEq(info1.sequenceNumber, requestCount + 1); + random.advanceProviderCommitment( + provider1, + updateSeqNumber, + provider1Proofs[updateSeqNumber] + ); + info1 = random.getProviderInfo(provider1); + assertEq(info1.currentCommitmentSequenceNumber, updateSeqNumber); + assertEq(info1.currentCommitment, provider1Proofs[updateSeqNumber]); + assertEq(info1.sequenceNumber, requestCount + 1); + assertInvariants(); + } + + function testAdvanceProviderCommitmentTooOld( + uint32 requestCount, + uint32 updateSeqNumber + ) public { + vm.assume(requestCount < provider1MaxNumHashes); + vm.assume(updateSeqNumber < requestCount); + vm.assume(0 < updateSeqNumber); + + for (uint256 i = 0; i < requestCount; i++) { + request(user1, provider1, 42, false); + } + assertRevealSucceeds( + user1, + provider1, + requestCount, + 42, + provider1Proofs[requestCount], + ALL_ZEROS + ); + vm.expectRevert(EntropyErrors.UpdateTooOld.selector); + random.advanceProviderCommitment( + provider1, + updateSeqNumber, + provider1Proofs[updateSeqNumber] + ); + } + + function testAdvanceProviderCommitmentIncorrectRevelation( + uint32 seqNumber, + uint32 mismatchedProofNumber + ) public { + vm.assume(seqNumber < provider1ChainLength); + vm.assume(mismatchedProofNumber < provider1ChainLength); + vm.assume(seqNumber != mismatchedProofNumber); + vm.assume(seqNumber > 0); + vm.expectRevert(EntropyErrors.IncorrectRevelation.selector); + random.advanceProviderCommitment( + provider1, + seqNumber, + provider1Proofs[mismatchedProofNumber] + ); + } + + function testAdvanceProviderCommitmentUpdatesSequenceNumber( + uint32 seqNumber + ) public { + vm.assume(seqNumber < provider1ChainLength); + vm.assume(seqNumber > 0); + random.advanceProviderCommitment( + provider1, + seqNumber, + provider1Proofs[seqNumber] + ); + EntropyStructs.ProviderInfo memory info1 = random.getProviderInfo( + provider1 + ); + assertEq(info1.sequenceNumber, seqNumber + 1); + } + + function testAdvanceProviderCommitmentHigherThanChainLength( + uint32 seqNumber + ) public { + vm.assume(seqNumber >= provider1ChainLength); + vm.expectRevert(EntropyErrors.AssertionFailure.selector); + random.advanceProviderCommitment( + provider1, + seqNumber, + provider1Proofs[0] + ); + } + + function testSetMaxNumHashes(uint32 maxNumHashes) public { + vm.prank(provider1); + random.setMaxNumHashes(maxNumHashes); + EntropyStructs.ProviderInfo memory info1 = random.getProviderInfo( + provider1 + ); + assertEq(info1.maxNumHashes, maxNumHashes); + } + + function testSetMaxNumHashesRevertIfNotFromProvider() public { + vm.expectRevert(EntropyErrors.NoSuchProvider.selector); + random.setMaxNumHashes(100); + } + + function testZeroMaxNumHashesDisableChecks() public { + for (uint256 i = 0; i < provider1MaxNumHashes; i++) { + request(user1, provider1, 42, false); + } + assertRequestReverts( + random.getFee(provider1), + provider1, + 42, + false, + EntropyErrors.LastRevealedTooOld.selector + ); + vm.prank(provider1); + random.setMaxNumHashes(0); + request(user1, provider1, 42, false); + } + function testFeeManager() public { address manager = address(12); diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol index 017409ee72..0511daae9a 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol @@ -38,4 +38,9 @@ library EntropyErrors { // else if a request was made using `request`, request should be fulfilled using `reveal` // Signature: 0x50f0dc92 error InvalidRevealCall(); + // The last random number revealed from the provider is too old. Therefore, too many hashes + // are required for any new reveal. Please update the currentCommitment before making more requests. + error LastRevealedTooOld(); + // A more recent commitment is already revealed on-chain + error UpdateTooOld(); } diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol index 0008031e24..59b595fb13 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol @@ -38,6 +38,11 @@ interface EntropyEvents { address oldFeeManager, address newFeeManager ); + event ProviderMaxNumHashesAdvanced( + address provider, + uint32 oldMaxNumHashes, + uint32 newMaxNumHashes + ); event Withdrawal( address provider, diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol index 4e5c9f79e4..15c1d1511a 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol @@ -34,6 +34,9 @@ contract EntropyStructs { uint64 currentCommitmentSequenceNumber; // An address that is authorized to set / withdraw fees on behalf of this provider. address feeManager; + // Maximum number of hashes to record in a request. This should be set according to the maximum gas limit + // the provider supports for callbacks. + uint32 maxNumHashes; } struct Request { diff --git a/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol b/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol index 2d9b6ac72f..ce446ff44b 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol @@ -120,6 +120,18 @@ interface IEntropy is EntropyEvents { // will override the previous value. Call this function with the all-zero address to disable the fee manager role. function setFeeManager(address manager) external; + // Set the maximum number of hashes to record in a request. This should be set according to the maximum gas limit + // the provider supports for callbacks. + function setMaxNumHashes(uint32 maxNumHashes) external; + + // Advance the provider commitment and increase the sequence number. + // This is used to reduce the `numHashes` required for future requests which leads to reduced gas usage. + function advanceProviderCommitment( + address provider, + uint64 advancedSequenceNumber, + bytes32 providerRevelation + ) external; + function constructUserCommitment( bytes32 userRandomness ) external pure returns (bytes32 userCommitment); diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json index d37ec7810a..24a6ef2548 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json @@ -29,6 +29,11 @@ "name": "InvalidUpgradeMagic", "type": "error" }, + { + "inputs": [], + "name": "LastRevealedTooOld", + "type": "error" + }, { "inputs": [], "name": "NoSuchProvider", @@ -53,5 +58,10 @@ "inputs": [], "name": "Unauthorized", "type": "error" + }, + { + "inputs": [], + "name": "UpdateTooOld", + "type": "error" } ] diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json index c149974ecd..b34dffcf59 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json @@ -49,6 +49,31 @@ "name": "ProviderFeeUpdated", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "oldMaxNumHashes", + "type": "uint32" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "newMaxNumHashes", + "type": "uint32" + } + ], + "name": "ProviderMaxNumHashesAdvanced", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -133,6 +158,11 @@ "internalType": "address", "name": "feeManager", "type": "address" + }, + { + "internalType": "uint32", + "name": "maxNumHashes", + "type": "uint32" } ], "indexed": false, diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json b/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json index a84e94b4ed..61a4a6be2e 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json @@ -49,6 +49,31 @@ "name": "ProviderFeeUpdated", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "oldMaxNumHashes", + "type": "uint32" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "newMaxNumHashes", + "type": "uint32" + } + ], + "name": "ProviderMaxNumHashesAdvanced", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -133,6 +158,11 @@ "internalType": "address", "name": "feeManager", "type": "address" + }, + { + "internalType": "uint32", + "name": "maxNumHashes", + "type": "uint32" } ], "indexed": false, @@ -455,6 +485,29 @@ "name": "Withdrawal", "type": "event" }, + { + "inputs": [ + { + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "internalType": "uint64", + "name": "advancedSequenceNumber", + "type": "uint64" + }, + { + "internalType": "bytes32", + "name": "providerRevelation", + "type": "bytes32" + } + ], + "name": "advanceProviderCommitment", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -614,6 +667,11 @@ "internalType": "address", "name": "feeManager", "type": "address" + }, + { + "internalType": "uint32", + "name": "maxNumHashes", + "type": "uint32" } ], "internalType": "struct EntropyStructs.ProviderInfo", @@ -851,6 +909,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint32", + "name": "maxNumHashes", + "type": "uint32" + } + ], + "name": "setMaxNumHashes", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ {