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

Author: m30m

apps/fortuna/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name    = "fortuna"
-version = "6.4.2"
+version = "6.5.2"
 edition = "2021"
 
 [dependencies]

apps/fortuna/Cargo.toml

@@ -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<T> = PythRandom<
-    LegacyTxMiddleware<
-        GasOracleMiddleware<
-            NonceManagerMiddleware<SignerMiddleware<Provider<T>, LocalWallet>>,
-            EthProviderOracle<Provider<T>>,
-        >,
+pub type MiddlewaresWrapper<T> = LegacyTxMiddleware<
+    GasOracleMiddleware<
+        NonceManagerMiddleware<SignerMiddleware<Provider<T>, LocalWallet>>,
+        EthProviderOracle<Provider<T>>,
     >,
 >;
+pub type SignablePythContractInner<T> = PythRandom<MiddlewaresWrapper<T>>;
 pub type SignablePythContract = SignablePythContractInner<Http>;
 pub type InstrumentedSignablePythContract = SignablePythContractInner<TracedClient>;
 
+pub type PythContractCall = ContractCall<MiddlewaresWrapper<TracedClient>, ()>;
+
 pub type PythContract = PythRandom<Provider<Http>>;
 pub type InstrumentedPythContract = PythRandom<Provider<TracedClient>>;
 

apps/fortuna/src/chain/ethereum.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<SignablePythContract>,
+    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(())
+}

apps/fortuna/src/command/setup_provider.rs

@@ -183,6 +183,10 @@ pub struct EthereumConfig {
 
     /// Historical commitments made by the provider.
     pub commitments: Option<Vec<Commitment>>,
+
+    /// 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<u32>,
 }
 
 

apps/fortuna/src/config.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,28 +967,46 @@ 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)
     }
 
     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<InstrumentedSignablePythContract>,
@@ -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<InstrumentedSignablePythContract>,
+    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<InstrumentedSignablePythContract>,
+    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 {

apps/fortuna/src/keeper.rs

@@ -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) {

target_chains/ethereum/contracts/contracts/entropy/Entropy.sol

@@ -105,6 +105,6 @@ contract EntropyUpgradable is
     }
 
     function version() public pure returns (string memory) {
-        return "0.3.1";
+        return "0.4.0";
     }
 }