From ed5476d534faec93e2b0ce2f0109966b4bb053b1 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Fri, 25 Apr 2025 10:23:31 -0700 Subject: [PATCH 1/7] upgrading to use the new contract API --- apps/fortuna/src/api/revelation.rs | 2 +- apps/fortuna/src/chain/ethereum.rs | 26 ++++------- apps/fortuna/src/chain/reader.rs | 4 +- apps/fortuna/src/command/get_request.rs | 4 +- apps/fortuna/src/command/inspect.rs | 14 +++--- apps/fortuna/src/command/run.rs | 2 +- apps/fortuna/src/command/setup_provider.rs | 51 +++++++++++++++------- apps/fortuna/src/command/withdraw_fees.rs | 2 +- apps/fortuna/src/config.rs | 2 +- apps/fortuna/src/keeper.rs | 2 +- apps/fortuna/src/keeper/commitment.rs | 2 +- apps/fortuna/src/keeper/fee.rs | 4 +- apps/fortuna/src/keeper/process_event.rs | 2 +- apps/fortuna/src/keeper/track.rs | 2 +- 14 files changed, 66 insertions(+), 53 deletions(-) diff --git a/apps/fortuna/src/api/revelation.rs b/apps/fortuna/src/api/revelation.rs index 152bd99132..814eaee61f 100644 --- a/apps/fortuna/src/api/revelation.rs +++ b/apps/fortuna/src/api/revelation.rs @@ -45,7 +45,7 @@ pub async fn revelation( .get(&chain_id) .ok_or(RestError::InvalidChainId)?; - let maybe_request_fut = state.contract.get_request(state.provider_address, sequence); + let maybe_request_fut = state.contract.get_request_v2(state.provider_address, sequence); let current_block_number_fut = state .contract diff --git a/apps/fortuna/src/chain/ethereum.rs b/apps/fortuna/src/chain/ethereum.rs index daae50a4ee..12c2516b34 100644 --- a/apps/fortuna/src/chain/ethereum.rs +++ b/apps/fortuna/src/chain/ethereum.rs @@ -233,29 +233,21 @@ impl InstrumentedPythContract { #[async_trait] impl EntropyReader for PythRandom> { - async fn get_request( + async fn get_request_v2( &self, provider_address: Address, sequence_number: u64, ) -> Result> { - let r = self - .get_request(provider_address, sequence_number) - // TODO: This doesn't work for lighlink right now. Figure out how to do this in lightlink - // .block(ethers::core::types::BlockNumber::Finalized) + let request = self + .get_request_v2(provider_address, sequence_number) .call() .await?; - - // sequence_number == 0 means the request does not exist. - if r.sequence_number != 0 { - Ok(Some(reader::Request { - provider: r.provider, - sequence_number: r.sequence_number, - block_number: r.block_number, - use_blockhash: r.use_blockhash, - })) - } else { - Ok(None) - } + Ok(Some(reader::Request { + provider: request.provider, + sequence_number: request.sequence_number, + block_number: request.block_number, + use_blockhash: request.use_blockhash, + })) } async fn get_block_number(&self, confirmed_block_status: BlockStatus) -> Result { diff --git a/apps/fortuna/src/chain/reader.rs b/apps/fortuna/src/chain/reader.rs index fde5fd48bc..c595dde6db 100644 --- a/apps/fortuna/src/chain/reader.rs +++ b/apps/fortuna/src/chain/reader.rs @@ -42,7 +42,7 @@ pub trait EntropyReader: Send + Sync { /// Get an in-flight request (if it exists) /// Note that if we support additional blockchains in the future, the type of `provider` may /// need to become more generic. - async fn get_request(&self, provider: Address, sequence_number: u64) + async fn get_request_v2(&self, provider: Address, sequence_number: u64) -> Result>; async fn get_block_number(&self, confirmed_block_status: BlockStatus) -> Result; @@ -141,7 +141,7 @@ pub mod mock { #[async_trait] impl EntropyReader for MockEntropyReader { - async fn get_request( + async fn get_request_v2( &self, provider: Address, sequence_number: u64, diff --git a/apps/fortuna/src/command/get_request.rs b/apps/fortuna/src/command/get_request.rs index 1f6e49bd2b..300b6a6896 100644 --- a/apps/fortuna/src/command/get_request.rs +++ b/apps/fortuna/src/command/get_request.rs @@ -14,12 +14,12 @@ pub async fn get_request(opts: &GetRequestOptions) -> Result<()> { &Config::load(&opts.config.config)?.get_chain_config(&opts.chain_id)?, )?); - let p = contract.get_provider_info(opts.provider).call().await?; + let p = contract.get_provider_info_v2(opts.provider).call().await?; tracing::info!("Found provider: {:?}", p); let r = contract - .get_request(opts.provider, opts.sequence) + .get_request_v2(opts.provider, opts.sequence) .call() .await?; tracing::info!("Found request: {:?}", r); diff --git a/apps/fortuna/src/command/inspect.rs b/apps/fortuna/src/command/inspect.rs index bc92856c0c..5c3c86f76b 100644 --- a/apps/fortuna/src/command/inspect.rs +++ b/apps/fortuna/src/command/inspect.rs @@ -1,6 +1,6 @@ use { crate::{ - chain::ethereum::{EntropyStructsRequest, PythContract}, + chain::ethereum::{EntropyStructsV2Request, PythContract}, config::{Config, EthereumConfig, InspectOptions}, }, anyhow::Result, @@ -43,7 +43,7 @@ async fn inspect_chain( let contract = PythContract::from_config(chain_config)?; let entropy_provider = contract.get_default_provider().call().await?; - let provider_info = contract.get_provider_info(entropy_provider).call().await?; + let provider_info = contract.get_provider_info_v2(entropy_provider).call().await?; let mut current_request_number = provider_info.sequence_number; println!("Initial request number: {}", current_request_number); let last_request_number = current_request_number.saturating_sub(num_requests); @@ -61,12 +61,12 @@ async fn inspect_chain( break; } multicall.add_call( - contract.get_request(entropy_provider, current_request_number), + contract.get_request_v2(entropy_provider, current_request_number), false, ); current_request_number -= 1; } - let return_data: Vec = multicall.call_array().await?; + let return_data: Vec = multicall.call_array().await?; for request in return_data { process_request(rpc_provider.clone(), request).await?; } @@ -76,7 +76,7 @@ async fn inspect_chain( println!("Multicall not deployed in this chain, fetching requests one by one"); while current_request_number > last_request_number { let request = contract - .get_request(entropy_provider, current_request_number) + .get_request_v2(entropy_provider, current_request_number) .call() .await?; process_request(rpc_provider.clone(), request).await?; @@ -91,9 +91,9 @@ async fn inspect_chain( async fn process_request( rpc_provider: Provider, - request: EntropyStructsRequest, + request: EntropyStructsV2Request, ) -> Result<()> { - if request.sequence_number != 0 && request.is_request_with_callback { + if request.sequence_number != 0 && request.callback_status != 0 { let block = rpc_provider .get_block(request.block_number) .await? diff --git a/apps/fortuna/src/command/run.rs b/apps/fortuna/src/command/run.rs index 128447b6b0..95f5be339e 100644 --- a/apps/fortuna/src/command/run.rs +++ b/apps/fortuna/src/command/run.rs @@ -228,7 +228,7 @@ async fn setup_chain_state( .cmp(&c2.original_commitment_sequence_number) }); - let provider_info = contract.get_provider_info(*provider).call().await?; + let provider_info = contract.get_provider_info_v2(*provider).call().await?; let latest_metadata = bincode::deserialize::( &provider_info.commitment_metadata, ) diff --git a/apps/fortuna/src/command/setup_provider.rs b/apps/fortuna/src/command/setup_provider.rs index 9b0e4db20a..283c156a9a 100644 --- a/apps/fortuna/src/command/setup_provider.rs +++ b/apps/fortuna/src/command/setup_provider.rs @@ -1,21 +1,15 @@ use { crate::{ api::{get_register_uri, ChainId}, - chain::ethereum::{EntropyStructsProviderInfo, SignablePythContract}, + chain::ethereum::{EntropyStructsV2ProviderInfo, SignablePythContract}, command::register_provider::{register_provider_from_config, CommitmentMetadata}, config::{Config, EthereumConfig, SetupProviderOptions}, state::{HashChainState, PebbleHashChain}, - }, - anyhow::{anyhow, Result}, - ethers::{ + }, anyhow::{anyhow, Result}, ethers::{ abi::Bytes as AbiBytes, signers::{LocalWallet, Signer}, types::{Address, Bytes}, - }, - futures::future::join_all, - std::sync::Arc, - tokio::spawn, - tracing::Instrument, + }, futures::future::join_all, std::sync::Arc, tokio::spawn, tracing::Instrument }; /// Setup provider for all the chains. @@ -76,7 +70,7 @@ async fn setup_chain_provider( let contract = Arc::new(SignablePythContract::from_config(chain_config, &private_key).await?); tracing::info!("Fetching provider info"); - let provider_info = contract.get_provider_info(provider_address).call().await?; + let provider_info = contract.get_provider_info_v2(provider_address).call().await?; tracing::info!("Provider info: {:?}", provider_info); let mut register = false; @@ -146,7 +140,7 @@ async fn setup_chain_provider( tracing::info!("Registered"); } - let provider_info = contract.get_provider_info(provider_address).call().await?; + let provider_info = contract.get_provider_info_v2(provider_address).call().await?; sync_fee(&contract, &provider_info, chain_config.fee) .in_current_span() @@ -173,12 +167,20 @@ async fn setup_chain_provider( .in_current_span() .await?; + sync_default_gas_limit( + &contract, + &provider_info, + chain_config.gas_limit, + ) + .in_current_span() + .await?; + Ok(()) } async fn sync_uri( contract: &Arc, - provider_info: &EntropyStructsProviderInfo, + provider_info: &EntropyStructsV2ProviderInfo, uri: String, ) -> Result<()> { let uri_as_bytes: Bytes = AbiBytes::from(uri.as_str()).into(); @@ -198,7 +200,7 @@ async fn sync_uri( async fn sync_fee( contract: &Arc, - provider_info: &EntropyStructsProviderInfo, + provider_info: &EntropyStructsV2ProviderInfo, provider_fee: u128, ) -> Result<()> { if provider_info.fee_in_wei != provider_fee { @@ -217,7 +219,7 @@ async fn sync_fee( async fn sync_fee_manager( contract: &Arc, - provider_info: &EntropyStructsProviderInfo, + provider_info: &EntropyStructsV2ProviderInfo, fee_manager: Address, ) -> Result<()> { if provider_info.fee_manager != fee_manager { @@ -231,7 +233,7 @@ async fn sync_fee_manager( async fn sync_max_num_hashes( contract: &Arc, - provider_info: &EntropyStructsProviderInfo, + provider_info: &EntropyStructsV2ProviderInfo, max_num_hashes: u32, ) -> Result<()> { if provider_info.max_num_hashes != max_num_hashes { @@ -247,3 +249,22 @@ async fn sync_max_num_hashes( } Ok(()) } + +async fn sync_default_gas_limit( + contract: &Arc, + provider_info: &EntropyStructsV2ProviderInfo, + default_gas_limit: u32, +) -> Result<()> { + if provider_info.default_gas_limit != default_gas_limit { + tracing::info!("Updating provider default gas limit to {:?}", default_gas_limit); + if let Some(receipt) = contract + .set_default_gas_limit(default_gas_limit) + .send() + .await? + .await? + { + tracing::info!("Updated provider default gas limit to : {:?}", receipt); + } + } + Ok(()) +} diff --git a/apps/fortuna/src/command/withdraw_fees.rs b/apps/fortuna/src/command/withdraw_fees.rs index 8f701823a9..1151545135 100644 --- a/apps/fortuna/src/command/withdraw_fees.rs +++ b/apps/fortuna/src/command/withdraw_fees.rs @@ -58,7 +58,7 @@ pub async fn withdraw_fees_for_chain( retained_balance: u128, ) -> Result<()> { tracing::info!("Fetching fees for provider: {:?}", provider_address); - let provider_info = contract.get_provider_info(provider_address).call().await?; + let provider_info = contract.get_provider_info_v2(provider_address).call().await?; let fees = provider_info.accrued_fees_in_wei; tracing::info!("Accrued fees: {} wei", fees); diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs index 5bf3582992..d332821075 100644 --- a/apps/fortuna/src/config.rs +++ b/apps/fortuna/src/config.rs @@ -136,7 +136,7 @@ pub struct EthereumConfig { pub legacy_tx: bool, /// The gas limit to use for entropy callback transactions. - pub gas_limit: u64, + pub gas_limit: u32, /// The percentage multiplier to apply to priority fee estimates (100 = no change, e.g. 150 = 150% of base fee) #[serde(default = "default_priority_fee_multiplier_pct")] diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index 3ade50b0ed..9e56f5ed4c 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -150,7 +150,7 @@ pub async fn run_keeper_threads( // near the maximum gas limit. // In the unlikely event that the keeper fees aren't sufficient, the solution to this is to configure the target // fee percentage to be higher on that specific chain. - chain_eth_config.gas_limit, + chain_eth_config.gas_limit.into(), // NOTE: unwrap() here so we panic early if someone configures these values below -100. u64::try_from(100 + chain_eth_config.min_profit_pct) .expect("min_profit_pct must be >= -100"), diff --git a/apps/fortuna/src/keeper/commitment.rs b/apps/fortuna/src/keeper/commitment.rs index c4c2344693..38040461c8 100644 --- a/apps/fortuna/src/keeper/commitment.rs +++ b/apps/fortuna/src/keeper/commitment.rs @@ -38,7 +38,7 @@ pub async fn update_commitments_if_necessary( 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) + .get_provider_info_v2(provider_address) .block(latest_safe_block) // To ensure we are not revealing sooner than we should .call() .await diff --git a/apps/fortuna/src/keeper/fee.rs b/apps/fortuna/src/keeper/fee.rs index 1e3c1e6dc3..d53d185fd7 100644 --- a/apps/fortuna/src/keeper/fee.rs +++ b/apps/fortuna/src/keeper/fee.rs @@ -48,7 +48,7 @@ pub async fn withdraw_fees_if_necessary( .map_err(|e| anyhow!("Error while getting balance. error: {:?}", e))?; let provider_info = contract - .get_provider_info(provider_address) + .get_provider_info_v2(provider_address) .call() .await .map_err(|e| anyhow!("Error while getting provider info. error: {:?}", e))?; @@ -142,7 +142,7 @@ pub async fn adjust_fee_if_necessary( metrics: Arc, ) -> Result<()> { let provider_info = contract - .get_provider_info(provider_address) + .get_provider_info_v2(provider_address) .call() .await .map_err(|e| anyhow!("Error while getting provider info. error: {:?}", e))?; diff --git a/apps/fortuna/src/keeper/process_event.rs b/apps/fortuna/src/keeper/process_event.rs index a4a6e51137..dd5e188ae7 100644 --- a/apps/fortuna/src/keeper/process_event.rs +++ b/apps/fortuna/src/keeper/process_event.rs @@ -119,7 +119,7 @@ pub async fn process_event_with_backoff( // the RPC gave us an error anyway. let req = chain_state .contract - .get_request(event.provider_address, event.sequence_number) + .get_request_v2(event.provider_address, event.sequence_number) .await; tracing::error!("Failed to process event: {:?}. Request: {:?}", e, req); diff --git a/apps/fortuna/src/keeper/track.rs b/apps/fortuna/src/keeper/track.rs index b31d4d8603..9c4347702e 100644 --- a/apps/fortuna/src/keeper/track.rs +++ b/apps/fortuna/src/keeper/track.rs @@ -50,7 +50,7 @@ pub async fn track_provider( provider_address: Address, metrics: Arc, ) { - let provider_info = match contract.get_provider_info(provider_address).call().await { + let provider_info = match contract.get_provider_info_v2(provider_address).call().await { Ok(info) => info, Err(e) => { tracing::error!("Error while getting provider info. error: {:?}", e); From c0444e0a54058efd67004b6332b5e653d337c44f Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Fri, 25 Apr 2025 11:16:13 -0700 Subject: [PATCH 2/7] deployable tester and contract testing --- apps/fortuna/src/api/revelation.rs | 4 +- apps/fortuna/src/chain/reader.rs | 7 +- apps/fortuna/src/command/inspect.rs | 5 +- apps/fortuna/src/command/setup_provider.rs | 35 ++++-- apps/fortuna/src/command/withdraw_fees.rs | 5 +- apps/fortuna/src/eth_utils/utils.rs | 2 + .../contracts/forge-test/Entropy.t.sol | 105 ++++-------------- 7 files changed, 64 insertions(+), 99 deletions(-) diff --git a/apps/fortuna/src/api/revelation.rs b/apps/fortuna/src/api/revelation.rs index 814eaee61f..817fe70760 100644 --- a/apps/fortuna/src/api/revelation.rs +++ b/apps/fortuna/src/api/revelation.rs @@ -45,7 +45,9 @@ pub async fn revelation( .get(&chain_id) .ok_or(RestError::InvalidChainId)?; - let maybe_request_fut = state.contract.get_request_v2(state.provider_address, sequence); + let maybe_request_fut = state + .contract + .get_request_v2(state.provider_address, sequence); let current_block_number_fut = state .contract diff --git a/apps/fortuna/src/chain/reader.rs b/apps/fortuna/src/chain/reader.rs index c595dde6db..570b970935 100644 --- a/apps/fortuna/src/chain/reader.rs +++ b/apps/fortuna/src/chain/reader.rs @@ -42,8 +42,11 @@ pub trait EntropyReader: Send + Sync { /// Get an in-flight request (if it exists) /// Note that if we support additional blockchains in the future, the type of `provider` may /// need to become more generic. - async fn get_request_v2(&self, provider: Address, sequence_number: u64) - -> Result>; + async fn get_request_v2( + &self, + provider: Address, + sequence_number: u64, + ) -> Result>; async fn get_block_number(&self, confirmed_block_status: BlockStatus) -> Result; diff --git a/apps/fortuna/src/command/inspect.rs b/apps/fortuna/src/command/inspect.rs index 5c3c86f76b..7a3f3a8ddb 100644 --- a/apps/fortuna/src/command/inspect.rs +++ b/apps/fortuna/src/command/inspect.rs @@ -43,7 +43,10 @@ async fn inspect_chain( let contract = PythContract::from_config(chain_config)?; let entropy_provider = contract.get_default_provider().call().await?; - let provider_info = contract.get_provider_info_v2(entropy_provider).call().await?; + let provider_info = contract + .get_provider_info_v2(entropy_provider) + .call() + .await?; let mut current_request_number = provider_info.sequence_number; println!("Initial request number: {}", current_request_number); let last_request_number = current_request_number.saturating_sub(num_requests); diff --git a/apps/fortuna/src/command/setup_provider.rs b/apps/fortuna/src/command/setup_provider.rs index 283c156a9a..f811f7d65e 100644 --- a/apps/fortuna/src/command/setup_provider.rs +++ b/apps/fortuna/src/command/setup_provider.rs @@ -5,11 +5,17 @@ use { command::register_provider::{register_provider_from_config, CommitmentMetadata}, config::{Config, EthereumConfig, SetupProviderOptions}, state::{HashChainState, PebbleHashChain}, - }, anyhow::{anyhow, Result}, ethers::{ + }, + anyhow::{anyhow, Result}, + ethers::{ abi::Bytes as AbiBytes, signers::{LocalWallet, Signer}, types::{Address, Bytes}, - }, futures::future::join_all, std::sync::Arc, tokio::spawn, tracing::Instrument + }, + futures::future::join_all, + std::sync::Arc, + tokio::spawn, + tracing::Instrument, }; /// Setup provider for all the chains. @@ -70,7 +76,10 @@ async fn setup_chain_provider( let contract = Arc::new(SignablePythContract::from_config(chain_config, &private_key).await?); tracing::info!("Fetching provider info"); - let provider_info = contract.get_provider_info_v2(provider_address).call().await?; + let provider_info = contract + .get_provider_info_v2(provider_address) + .call() + .await?; tracing::info!("Provider info: {:?}", provider_info); let mut register = false; @@ -140,7 +149,10 @@ async fn setup_chain_provider( tracing::info!("Registered"); } - let provider_info = contract.get_provider_info_v2(provider_address).call().await?; + let provider_info = contract + .get_provider_info_v2(provider_address) + .call() + .await?; sync_fee(&contract, &provider_info, chain_config.fee) .in_current_span() @@ -167,13 +179,9 @@ async fn setup_chain_provider( .in_current_span() .await?; - sync_default_gas_limit( - &contract, - &provider_info, - chain_config.gas_limit, - ) - .in_current_span() - .await?; + sync_default_gas_limit(&contract, &provider_info, chain_config.gas_limit) + .in_current_span() + .await?; Ok(()) } @@ -256,7 +264,10 @@ async fn sync_default_gas_limit( default_gas_limit: u32, ) -> Result<()> { if provider_info.default_gas_limit != default_gas_limit { - tracing::info!("Updating provider default gas limit to {:?}", default_gas_limit); + tracing::info!( + "Updating provider default gas limit to {:?}", + default_gas_limit + ); if let Some(receipt) = contract .set_default_gas_limit(default_gas_limit) .send() diff --git a/apps/fortuna/src/command/withdraw_fees.rs b/apps/fortuna/src/command/withdraw_fees.rs index 1151545135..fd69f10af4 100644 --- a/apps/fortuna/src/command/withdraw_fees.rs +++ b/apps/fortuna/src/command/withdraw_fees.rs @@ -58,7 +58,10 @@ pub async fn withdraw_fees_for_chain( retained_balance: u128, ) -> Result<()> { tracing::info!("Fetching fees for provider: {:?}", provider_address); - let provider_info = contract.get_provider_info_v2(provider_address).call().await?; + let provider_info = contract + .get_provider_info_v2(provider_address) + .call() + .await?; let fees = provider_info.accrued_fees_in_wei; tracing::info!("Accrued fees: {} wei", fees); diff --git a/apps/fortuna/src/eth_utils/utils.rs b/apps/fortuna/src/eth_utils/utils.rs index 7627cf701e..ec47e609b1 100644 --- a/apps/fortuna/src/eth_utils/utils.rs +++ b/apps/fortuna/src/eth_utils/utils.rs @@ -233,6 +233,7 @@ pub async fn submit_tx( // The gas limit on the simulated transaction is the maximum expected tx gas estimate, // but we are willing to pad the gas a bit to ensure reliable submission. + /* if gas_estimate > gas_limit { return Err(backoff::Error::permanent(anyhow!( "Gas estimate for reveal with callback is higher than the gas limit {} > {}", @@ -240,6 +241,7 @@ pub async fn submit_tx( gas_limit ))); } + */ // Pad the gas estimate after checking it against the simulation gas limit. let gas_estimate = gas_estimate.saturating_mul(gas_estimate_multiplier_pct.into()) / 100; diff --git a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol index 84fd32f59e..bc72b9b9d6 100644 --- a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol @@ -11,6 +11,7 @@ import "@pythnetwork/entropy-sdk-solidity/IEntropy.sol"; import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import "./utils/EntropyTestUtils.t.sol"; import "../contracts/entropy/EntropyUpgradable.sol"; +import "../contracts/entropy/EntropyGasTester.sol"; import "@pythnetwork/entropy-sdk-solidity/EntropyStatusConstants.sol"; // TODO @@ -846,7 +847,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { function testRequestWithCallbackAndRevealWithCallbackByContract() public { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), false); + EntropyGasTester consumer = new EntropyGasTester( + address(random), + false + ); vm.deal(user1, fee); vm.prank(user1); uint64 assignedSequenceNumber = consumer.requestEntropy{value: fee}( @@ -955,7 +959,7 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { function testRequestWithCallbackAndRevealWithCallbackFailing() public { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), true); + EntropyGasTester consumer = new EntropyGasTester(address(random), true); vm.deal(address(consumer), fee); vm.startPrank(address(consumer)); uint64 assignedSequenceNumber = random.requestWithCallback{value: fee}( @@ -979,7 +983,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), false); + EntropyGasTester consumer = new EntropyGasTester( + address(random), + false + ); vm.deal(user1, fee); vm.prank(user1); uint64 assignedSequenceNumber = consumer.requestEntropy{value: fee}( @@ -1036,7 +1043,7 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), true); + EntropyGasTester consumer = new EntropyGasTester(address(random), true); vm.deal(user1, fee); vm.prank(user1); uint64 assignedSequenceNumber = consumer.requestEntropy{value: fee}( @@ -1135,7 +1142,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), false); + EntropyGasTester consumer = new EntropyGasTester( + address(random), + false + ); // Consumer callback uses ~10% more gas than the provider's default consumer.setTargetGasUsage((defaultGasLimit * 110) / 100); @@ -1256,7 +1266,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); - EntropyConsumer consumer = new EntropyConsumer(address(random), false); + EntropyGasTester consumer = new EntropyGasTester( + address(random), + false + ); consumer.setTargetGasUsage(defaultGasLimit); vm.deal(user1, fee); @@ -1659,7 +1672,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { vm.deal(user1, fee); vm.prank(user1); - EntropyConsumer consumer = new EntropyConsumer(address(random), false); + EntropyGasTester consumer = new EntropyGasTester( + address(random), + false + ); uint64 sequenceNumber = consumer.requestEntropyWithGasLimit{value: fee}( userRandomNumber, gasLimit @@ -1729,78 +1745,3 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { } } } - -contract EntropyConsumer is IEntropyConsumer { - uint64 public sequence; - bytes32 public randomness; - address public provider; - address public entropy; - bool public reverts; - uint256 public targetGasUsage; - - constructor(address _entropy, bool _reverts) { - entropy = _entropy; - reverts = _reverts; - targetGasUsage = 0; // Default target - } - - function requestEntropy( - bytes32 randomNumber - ) public payable returns (uint64 sequenceNumber) { - address _provider = IEntropy(entropy).getDefaultProvider(); - sequenceNumber = IEntropy(entropy).requestWithCallback{ - value: msg.value - }(_provider, randomNumber); - } - - function requestEntropyWithGasLimit( - bytes32 randomNumber, - uint32 gasLimit - ) public payable returns (uint64 sequenceNumber) { - address _provider = IEntropy(entropy).getDefaultProvider(); - sequenceNumber = IEntropy(entropy).requestWithCallbackAndGasLimit{ - value: msg.value - }(_provider, randomNumber, gasLimit); - } - - function getEntropy() internal view override returns (address) { - return entropy; - } - - function setReverts(bool _reverts) public { - reverts = _reverts; - } - - function setTargetGasUsage(uint256 _targetGasUsage) public { - require( - _targetGasUsage > 60000, - "Target gas usage cannot be below 60k (~the cost of storing callback results)" - ); - targetGasUsage = _targetGasUsage; - } - - function entropyCallback( - uint64 _sequence, - address _provider, - bytes32 _randomness - ) internal override { - uint256 startGas = gasleft(); - // These seemingly innocuous instructions are actually quite expensive - // (~60k gas) because they're writes to contract storage. - sequence = _sequence; - provider = _provider; - randomness = _randomness; - - // Keep consuming gas until we reach our target - uint256 currentGasUsed = startGas - gasleft(); - while (currentGasUsed < targetGasUsage) { - // Consume gas with a hash operation - keccak256(abi.encodePacked(currentGasUsed, _randomness)); - currentGasUsed = startGas - gasleft(); - } - - if (reverts) { - revert("Callback failed"); - } - } -} From b6dc863de16f151c0db6e12a9ae97779913c8e22 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Thu, 15 May 2025 13:38:41 -0700 Subject: [PATCH 3/7] making this work on the new contracts --- apps/fortuna/Cargo.lock | 2 +- apps/fortuna/Cargo.toml | 2 +- apps/fortuna/src/config.rs | 41 ---------------- apps/fortuna/src/eth_utils/utils.rs | 60 +---------------------- apps/fortuna/src/keeper/keeper_metrics.rs | 13 ----- apps/fortuna/src/keeper/process_event.rs | 8 +-- 6 files changed, 5 insertions(+), 121 deletions(-) diff --git a/apps/fortuna/Cargo.lock b/apps/fortuna/Cargo.lock index 4573c11dec..d21a84bcee 100644 --- a/apps/fortuna/Cargo.lock +++ b/apps/fortuna/Cargo.lock @@ -1554,7 +1554,7 @@ dependencies = [ [[package]] name = "fortuna" -version = "7.5.3" +version = "8.0.0" dependencies = [ "anyhow", "axum", diff --git a/apps/fortuna/Cargo.toml b/apps/fortuna/Cargo.toml index a4491df717..99be2e0fbd 100644 --- a/apps/fortuna/Cargo.toml +++ b/apps/fortuna/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fortuna" -version = "7.5.3" +version = "8.0.0" edition = "2021" [lib] diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs index e13a81d3af..d54db7c2f8 100644 --- a/apps/fortuna/src/config.rs +++ b/apps/fortuna/src/config.rs @@ -200,23 +200,6 @@ fn default_backlog_range() -> u64 { #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct EscalationPolicyConfig { - // The keeper will perform the callback as long as the tx is within this percentage of the configured gas limit. - // Default value is 110, meaning a 10% tolerance over the configured value. - #[serde(default = "default_gas_limit_tolerance_pct")] - pub gas_limit_tolerance_pct: u64, - - /// The initial gas multiplier to apply to the tx gas estimate - #[serde(default = "default_initial_gas_multiplier_pct")] - pub initial_gas_multiplier_pct: u64, - - /// The gas multiplier to apply to the tx gas estimate during backoff retries. - /// The gas on each successive retry is multiplied by this value, with the maximum multiplier capped at `gas_multiplier_cap_pct`. - #[serde(default = "default_gas_multiplier_pct")] - pub gas_multiplier_pct: u64, - /// The maximum gas multiplier to apply to the tx gas estimate during backoff retries. - #[serde(default = "default_gas_multiplier_cap_pct")] - pub gas_multiplier_cap_pct: u64, - /// The fee multiplier to apply to the fee during backoff retries. /// The initial fee is 100% of the estimate (which itself may be padded based on our chain configuration) /// The fee on each successive retry is multiplied by this value, with the maximum multiplier capped at `fee_multiplier_cap_pct`. @@ -226,22 +209,6 @@ pub struct EscalationPolicyConfig { pub fee_multiplier_cap_pct: u64, } -fn default_gas_limit_tolerance_pct() -> u64 { - 110 -} - -fn default_initial_gas_multiplier_pct() -> u64 { - 125 -} - -fn default_gas_multiplier_pct() -> u64 { - 110 -} - -fn default_gas_multiplier_cap_pct() -> u64 { - 600 -} - fn default_fee_multiplier_pct() -> u64 { 110 } @@ -253,10 +220,6 @@ fn default_fee_multiplier_cap_pct() -> u64 { impl Default for EscalationPolicyConfig { fn default() -> Self { Self { - gas_limit_tolerance_pct: default_gas_limit_tolerance_pct(), - initial_gas_multiplier_pct: default_initial_gas_multiplier_pct(), - gas_multiplier_pct: default_gas_multiplier_pct(), - gas_multiplier_cap_pct: default_gas_multiplier_cap_pct(), fee_multiplier_pct: default_fee_multiplier_pct(), fee_multiplier_cap_pct: default_fee_multiplier_cap_pct(), } @@ -266,10 +229,6 @@ impl Default for EscalationPolicyConfig { impl EscalationPolicyConfig { pub fn to_policy(&self) -> EscalationPolicy { EscalationPolicy { - gas_limit_tolerance_pct: self.gas_limit_tolerance_pct, - initial_gas_multiplier_pct: self.initial_gas_multiplier_pct, - gas_multiplier_pct: self.gas_multiplier_pct, - gas_multiplier_cap_pct: self.gas_multiplier_cap_pct, fee_multiplier_pct: self.fee_multiplier_pct, fee_multiplier_cap_pct: self.fee_multiplier_cap_pct, } diff --git a/apps/fortuna/src/eth_utils/utils.rs b/apps/fortuna/src/eth_utils/utils.rs index ec47e609b1..e9b07394f5 100644 --- a/apps/fortuna/src/eth_utils/utils.rs +++ b/apps/fortuna/src/eth_utils/utils.rs @@ -6,7 +6,7 @@ use { ethers::{ contract::ContractCall, middleware::Middleware, - types::{TransactionReceipt, U256}, + types::TransactionReceipt, }, std::sync::{atomic::AtomicU64, Arc}, tokio::time::{timeout, Duration}, @@ -18,7 +18,6 @@ const TX_CONFIRMATION_TIMEOUT_SECS: u64 = 30; #[derive(Debug)] pub struct SubmitTxResult { pub num_retries: u64, - pub gas_multiplier: u64, pub fee_multiplier: u64, pub duration: Duration, pub receipt: TransactionReceipt, @@ -26,19 +25,6 @@ pub struct SubmitTxResult { #[derive(Clone, Debug)] pub struct EscalationPolicy { - // The keeper will perform the callback as long as the tx is within this percentage of the configured gas limit. - // Default value is 110, meaning a 10% tolerance over the configured value. - pub gas_limit_tolerance_pct: u64, - - /// The initial gas multiplier to apply to the tx gas estimate - pub initial_gas_multiplier_pct: u64, - - /// The gas multiplier to apply to the tx gas estimate during backoff retries. - /// The gas on each successive retry is multiplied by this value, with the maximum multiplier capped at `gas_multiplier_cap_pct`. - pub gas_multiplier_pct: u64, - /// The maximum gas multiplier to apply to the tx gas estimate during backoff retries. - pub gas_multiplier_cap_pct: u64, - /// The fee multiplier to apply to the fee during backoff retries. /// The initial fee is 100% of the estimate (which itself may be padded based on our chain configuration) /// The fee on each successive retry is multiplied by this value, with the maximum multiplier capped at `fee_multiplier_cap_pct`. @@ -47,15 +33,6 @@ pub struct EscalationPolicy { } impl EscalationPolicy { - pub fn get_gas_multiplier_pct(&self, num_retries: u64) -> u64 { - self.apply_escalation_policy( - num_retries, - self.initial_gas_multiplier_pct, - self.gas_multiplier_pct, - self.gas_multiplier_cap_pct, - ) - } - pub fn get_fee_multiplier_pct(&self, num_retries: u64) -> u64 { self.apply_escalation_policy( num_retries, @@ -154,7 +131,6 @@ pub async fn estimate_tx_cost( pub async fn submit_tx_with_backoff( middleware: Arc, call: ContractCall, - gas_limit: U256, escalation_policy: EscalationPolicy, ) -> Result { let start_time = std::time::Instant::now(); @@ -167,20 +143,15 @@ pub async fn submit_tx_with_backoff( let num_retries = Arc::new(AtomicU64::new(0)); - let padded_gas_limit = U256::from(escalation_policy.gas_limit_tolerance_pct) * gas_limit / 100; - let success = backoff::future::retry_notify( backoff, || async { let num_retries = num_retries.load(std::sync::atomic::Ordering::Relaxed); - let gas_multiplier_pct = escalation_policy.get_gas_multiplier_pct(num_retries); let fee_multiplier_pct = escalation_policy.get_fee_multiplier_pct(num_retries); submit_tx( middleware.clone(), &call, - padded_gas_limit, - gas_multiplier_pct, fee_multiplier_pct, ) .await @@ -203,7 +174,6 @@ pub async fn submit_tx_with_backoff( Ok(SubmitTxResult { num_retries, - gas_multiplier: escalation_policy.get_gas_multiplier_pct(num_retries), fee_multiplier: escalation_policy.get_fee_multiplier_pct(num_retries), duration, receipt: success, @@ -217,36 +187,10 @@ pub async fn submit_tx_with_backoff( pub async fn submit_tx( client: Arc, call: &ContractCall, - gas_limit: U256, - // A value of 100 submits the tx with the same gas/fee as the estimate. - gas_estimate_multiplier_pct: u64, + // A value of 100 submits the tx with the same fee as the estimate. fee_estimate_multiplier_pct: u64, ) -> Result> { - let gas_estimate_res = call.estimate_gas().await; - - let gas_estimate = gas_estimate_res.map_err(|e| { - // we consider the error transient even if it is a contract revert since - // it can be because of routing to a lagging RPC node. Retrying such errors will - // incur a few additional RPC calls, but it is fine. - backoff::Error::transient(anyhow!("Error estimating gas for reveal: {:?}", e)) - })?; - - // The gas limit on the simulated transaction is the maximum expected tx gas estimate, - // but we are willing to pad the gas a bit to ensure reliable submission. - /* - if gas_estimate > gas_limit { - return Err(backoff::Error::permanent(anyhow!( - "Gas estimate for reveal with callback is higher than the gas limit {} > {}", - gas_estimate, - gas_limit - ))); - } - */ - - // Pad the gas estimate after checking it against the simulation gas limit. - let gas_estimate = gas_estimate.saturating_mul(gas_estimate_multiplier_pct.into()) / 100; - let call = call.clone().gas(gas_estimate); let mut transaction = call.tx.clone(); // manually fill the tx with the gas info, so we can log the details in case of error diff --git a/apps/fortuna/src/keeper/keeper_metrics.rs b/apps/fortuna/src/keeper/keeper_metrics.rs index abccb8abba..9334a66819 100644 --- a/apps/fortuna/src/keeper/keeper_metrics.rs +++ b/apps/fortuna/src/keeper/keeper_metrics.rs @@ -39,7 +39,6 @@ pub struct KeeperMetrics { pub reveals: Family, pub request_duration_ms: Family, pub retry_count: Family, - pub final_gas_multiplier: Family, pub final_fee_multiplier: Family, pub gas_price_estimate: Family>, pub accrued_pyth_fees: Family>, @@ -76,11 +75,6 @@ impl Default for KeeperMetrics { retry_count: Family::new_with_constructor(|| { Histogram::new(vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 10.0, 15.0, 20.0].into_iter()) }), - final_gas_multiplier: Family::new_with_constructor(|| { - Histogram::new( - vec![100.0, 125.0, 150.0, 200.0, 300.0, 400.0, 500.0, 600.0].into_iter(), - ) - }), final_fee_multiplier: Family::new_with_constructor(|| { Histogram::new(vec![100.0, 110.0, 120.0, 140.0, 160.0, 180.0, 200.0].into_iter()) }), @@ -198,12 +192,6 @@ impl KeeperMetrics { keeper_metrics.retry_count.clone(), ); - writable_registry.register( - "final_gas_multiplier", - "Final gas multiplier percentage for successful transactions", - keeper_metrics.final_gas_multiplier.clone(), - ); - writable_registry.register( "final_fee_multiplier", "Final fee multiplier percentage for successful transactions", @@ -270,7 +258,6 @@ impl KeeperMetrics { let _ = self.reveals.get_or_create(&account_label); let _ = self.request_duration_ms.get_or_create(&account_label); let _ = self.retry_count.get_or_create(&account_label); - let _ = self.final_gas_multiplier.get_or_create(&account_label); let _ = self.final_fee_multiplier.get_or_create(&account_label); let _ = self.gas_price_estimate.get_or_create(&account_label); } diff --git a/apps/fortuna/src/keeper/process_event.rs b/apps/fortuna/src/keeper/process_event.rs index dd5e188ae7..293244f722 100644 --- a/apps/fortuna/src/keeper/process_event.rs +++ b/apps/fortuna/src/keeper/process_event.rs @@ -19,7 +19,7 @@ pub async fn process_event_with_backoff( event: RequestedWithCallbackEvent, chain_state: BlockchainState, contract: Arc, - gas_limit: U256, + _gas_limit: U256, escalation_policy: EscalationPolicy, metrics: Arc, ) -> Result<()> { @@ -51,7 +51,6 @@ pub async fn process_event_with_backoff( let success = submit_tx_with_backoff( contract.client(), contract_call, - gas_limit, escalation_policy, ) .await; @@ -86,11 +85,6 @@ pub async fn process_event_with_backoff( .get_or_create(&account_label) .observe(result.num_retries as f64); - metrics - .final_gas_multiplier - .get_or_create(&account_label) - .observe(result.gas_multiplier as f64); - metrics .final_fee_multiplier .get_or_create(&account_label) From 1126b397df86e02d1678d3074deca84273d2352b Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Thu, 15 May 2025 13:57:49 -0700 Subject: [PATCH 4/7] cleanup --- apps/fortuna/src/eth_utils/utils.rs | 14 ++------------ apps/fortuna/src/keeper.rs | 3 --- apps/fortuna/src/keeper/block.rs | 11 ----------- apps/fortuna/src/keeper/process_event.rs | 9 +-------- 4 files changed, 3 insertions(+), 34 deletions(-) diff --git a/apps/fortuna/src/eth_utils/utils.rs b/apps/fortuna/src/eth_utils/utils.rs index e9b07394f5..8a2c712651 100644 --- a/apps/fortuna/src/eth_utils/utils.rs +++ b/apps/fortuna/src/eth_utils/utils.rs @@ -3,11 +3,7 @@ use { crate::eth_utils::nonce_manager::NonceManaged, anyhow::{anyhow, Result}, backoff::ExponentialBackoff, - ethers::{ - contract::ContractCall, - middleware::Middleware, - types::TransactionReceipt, - }, + ethers::{contract::ContractCall, middleware::Middleware, types::TransactionReceipt}, std::sync::{atomic::AtomicU64, Arc}, tokio::time::{timeout, Duration}, tracing, @@ -149,12 +145,7 @@ pub async fn submit_tx_with_backoff( let num_retries = num_retries.load(std::sync::atomic::Ordering::Relaxed); let fee_multiplier_pct = escalation_policy.get_fee_multiplier_pct(num_retries); - submit_tx( - middleware.clone(), - &call, - fee_multiplier_pct, - ) - .await + submit_tx(middleware.clone(), &call, fee_multiplier_pct).await }, |e, dur| { let retry_number = num_retries.load(std::sync::atomic::Ordering::Relaxed); @@ -190,7 +181,6 @@ pub async fn submit_tx( // A value of 100 submits the tx with the same fee as the estimate. fee_estimate_multiplier_pct: u64, ) -> Result> { - let mut transaction = call.tx.clone(); // manually fill the tx with the gas info, so we can log the details in case of error diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index 49ecec99c5..ef328fde60 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -79,7 +79,6 @@ pub async fn run_keeper_threads( let fulfilled_requests_cache = Arc::new(RwLock::new(HashSet::::new())); // Spawn a thread to handle the events from last backlog_range blocks. - let gas_limit: U256 = chain_eth_config.gas_limit.into(); spawn( process_backlog( BlockRange { @@ -87,7 +86,6 @@ pub async fn run_keeper_threads( to: latest_safe_block, }, contract.clone(), - gas_limit, chain_eth_config.escalation_policy.to_policy(), chain_state.clone(), metrics.clone(), @@ -107,7 +105,6 @@ pub async fn run_keeper_threads( chain_state.clone(), rx, Arc::clone(&contract), - gas_limit, chain_eth_config.escalation_policy.to_policy(), metrics.clone(), fulfilled_requests_cache.clone(), diff --git a/apps/fortuna/src/keeper/block.rs b/apps/fortuna/src/keeper/block.rs index c73719039b..6571894f93 100644 --- a/apps/fortuna/src/keeper/block.rs +++ b/apps/fortuna/src/keeper/block.rs @@ -7,7 +7,6 @@ use { keeper::process_event::process_event_with_backoff, }, anyhow::Result, - ethers::types::U256, std::{collections::HashSet, sync::Arc}, tokio::{ spawn, @@ -62,7 +61,6 @@ pub async fn get_latest_safe_block(chain_state: &BlockchainState) -> BlockNumber pub async fn process_block_range( block_range: BlockRange, contract: Arc, - gas_limit: U256, escalation_policy: EscalationPolicy, chain_state: api::BlockchainState, metrics: Arc, @@ -86,7 +84,6 @@ pub async fn process_block_range( to: to_block, }, contract.clone(), - gas_limit, escalation_policy.clone(), chain_state.clone(), metrics.clone(), @@ -109,7 +106,6 @@ pub async fn process_block_range( pub async fn process_single_block_batch( block_range: BlockRange, contract: Arc, - gas_limit: U256, escalation_policy: EscalationPolicy, chain_state: api::BlockchainState, metrics: Arc, @@ -140,7 +136,6 @@ pub async fn process_single_block_batch( event.clone(), chain_state.clone(), contract.clone(), - gas_limit, escalation_policy.clone(), metrics.clone(), ) @@ -251,7 +246,6 @@ pub async fn process_new_blocks( chain_state: BlockchainState, mut rx: mpsc::Receiver, contract: Arc, - gas_limit: U256, escalation_policy: EscalationPolicy, metrics: Arc, fulfilled_requests_cache: Arc>>, @@ -264,7 +258,6 @@ pub async fn process_new_blocks( process_block_range( block_range.clone(), Arc::clone(&contract), - gas_limit, escalation_policy.clone(), chain_state.clone(), metrics.clone(), @@ -282,7 +275,6 @@ pub async fn process_new_blocks( process_block_range( adjusted_range, Arc::clone(&contract), - gas_limit, escalation_policy.clone(), chain_state.clone(), metrics.clone(), @@ -302,7 +294,6 @@ pub async fn process_new_blocks( pub async fn process_backlog( backlog_range: BlockRange, contract: Arc, - gas_limit: U256, escalation_policy: EscalationPolicy, chain_state: BlockchainState, metrics: Arc, @@ -314,7 +305,6 @@ pub async fn process_backlog( process_block_range( backlog_range.clone(), Arc::clone(&contract), - gas_limit, escalation_policy.clone(), chain_state.clone(), metrics.clone(), @@ -332,7 +322,6 @@ pub async fn process_backlog( process_block_range( adjusted_range, Arc::clone(&contract), - gas_limit, escalation_policy.clone(), chain_state.clone(), metrics.clone(), diff --git a/apps/fortuna/src/keeper/process_event.rs b/apps/fortuna/src/keeper/process_event.rs index 293244f722..079d3c8a62 100644 --- a/apps/fortuna/src/keeper/process_event.rs +++ b/apps/fortuna/src/keeper/process_event.rs @@ -6,7 +6,6 @@ use { eth_utils::utils::{submit_tx_with_backoff, EscalationPolicy}, }, anyhow::{anyhow, Result}, - ethers::types::U256, std::sync::Arc, tracing, }; @@ -19,7 +18,6 @@ pub async fn process_event_with_backoff( event: RequestedWithCallbackEvent, chain_state: BlockchainState, contract: Arc, - _gas_limit: U256, escalation_policy: EscalationPolicy, metrics: Arc, ) -> Result<()> { @@ -48,12 +46,7 @@ pub async fn process_event_with_backoff( provider_revelation, ); - let success = submit_tx_with_backoff( - contract.client(), - contract_call, - escalation_policy, - ) - .await; + let success = submit_tx_with_backoff(contract.client(), contract_call, escalation_policy).await; metrics .requests_processed From f54b162ec9822bdcc0bf2301aa3da335e94d7b51 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Thu, 15 May 2025 14:03:40 -0700 Subject: [PATCH 5/7] cleanup another thing --- apps/fortuna/src/keeper.rs | 8 -------- apps/fortuna/src/keeper/fee.rs | 8 +++----- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index ef328fde60..4161c25656 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -132,14 +132,6 @@ pub async fn run_keeper_threads( chain_state.provider_address, ADJUST_FEE_INTERVAL, chain_eth_config.legacy_tx, - // NOTE: we are adjusting the fees based on the maximum configured gas for user transactions. - // However, the keeper will pad the gas limit for transactions (per the escalation policy) to ensure reliable submission. - // Consequently, fees can be adjusted such that transactions are still unprofitable. - // While we could scale up this value based on the padding, that ends up overcharging users as most transactions cost nowhere - // near the maximum gas limit. - // In the unlikely event that the keeper fees aren't sufficient, the solution to this is to configure the target - // fee percentage to be higher on that specific chain. - chain_eth_config.gas_limit.into(), // NOTE: unwrap() here so we panic early if someone configures these values below -100. u64::try_from(100 + chain_eth_config.min_profit_pct) .expect("min_profit_pct must be >= -100"), diff --git a/apps/fortuna/src/keeper/fee.rs b/apps/fortuna/src/keeper/fee.rs index 2ff8da4958..4fe2a4242f 100644 --- a/apps/fortuna/src/keeper/fee.rs +++ b/apps/fortuna/src/keeper/fee.rs @@ -79,7 +79,6 @@ pub async fn adjust_fee_wrapper( provider_address: Address, poll_interval: Duration, legacy_tx: bool, - gas_limit: u64, min_profit_pct: u64, target_profit_pct: u64, max_profit_pct: u64, @@ -96,7 +95,6 @@ pub async fn adjust_fee_wrapper( chain_state.id.clone(), provider_address, legacy_tx, - gas_limit, min_profit_pct, target_profit_pct, max_profit_pct, @@ -133,7 +131,6 @@ pub async fn adjust_fee_if_necessary( chain_id: ChainId, provider_address: Address, legacy_tx: bool, - gas_limit: u64, min_profit_pct: u64, target_profit_pct: u64, max_profit_pct: u64, @@ -154,7 +151,8 @@ pub async fn adjust_fee_if_necessary( // Calculate target window for the on-chain fee. let middleware = contract.client(); - let max_callback_cost: u128 = estimate_tx_cost(middleware, legacy_tx, gas_limit.into()) + let gas_limit: u128 = u128::from(provider_info.default_gas_limit); + let max_callback_cost: u128 = estimate_tx_cost(middleware, legacy_tx, gas_limit) .await .map_err(|e| anyhow!("Could not estimate transaction cost. error {:?}", e))?; @@ -166,7 +164,7 @@ pub async fn adjust_fee_if_necessary( metrics .gas_price_estimate .get_or_create(&account_label) - .set((max_callback_cost / u128::from(gas_limit)) as f64 / 1e9); + .set((max_callback_cost / gas_limit) as f64 / 1e9); let target_fee_min = std::cmp::max( (max_callback_cost * u128::from(min_profit_pct)) / 100, From e874d8a848f32e8cc381de19159295300728c4a3 Mon Sep 17 00:00:00 2001 From: Tejas Badadare Date: Mon, 7 Jul 2025 14:01:25 -0700 Subject: [PATCH 6/7] check callback status in replica logic --- apps/fortuna/src/chain/ethereum.rs | 1 + apps/fortuna/src/chain/reader.rs | 40 +++++++++++++++++++++++- apps/fortuna/src/keeper/process_event.rs | 19 ++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/apps/fortuna/src/chain/ethereum.rs b/apps/fortuna/src/chain/ethereum.rs index c2187e242a..fe6464ce85 100644 --- a/apps/fortuna/src/chain/ethereum.rs +++ b/apps/fortuna/src/chain/ethereum.rs @@ -279,6 +279,7 @@ impl EntropyReader for PythRandom> { sequence_number: request.sequence_number, block_number: request.block_number, use_blockhash: request.use_blockhash, + callback_status: reader::RequestCallbackStatus::try_from(request.callback_status)?, })) } diff --git a/apps/fortuna/src/chain/reader.rs b/apps/fortuna/src/chain/reader.rs index 49b39aa2e3..44130f85c3 100644 --- a/apps/fortuna/src/chain/reader.rs +++ b/apps/fortuna/src/chain/reader.rs @@ -96,12 +96,48 @@ pub struct Request { // The block number where this request was created pub block_number: BlockNumber, pub use_blockhash: bool, + pub callback_status: RequestCallbackStatus, +} + +/// Status values for Request.callback_status +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum RequestCallbackStatus { + /// Not a request with callback + CallbackNotNecessary = 0, + /// A request with callback where the callback hasn't been invoked yet + CallbackNotStarted = 1, + /// A request with callback where the callback is currently in flight (this state is a reentry guard) + CallbackInProgress = 2, + /// A request with callback where the callback has been invoked and failed + CallbackFailed = 3, +} + +impl TryFrom for RequestCallbackStatus { + type Error = anyhow::Error; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(RequestCallbackStatus::CallbackNotNecessary), + 1 => Ok(RequestCallbackStatus::CallbackNotStarted), + 2 => Ok(RequestCallbackStatus::CallbackInProgress), + 3 => Ok(RequestCallbackStatus::CallbackFailed), + _ => Err(anyhow::anyhow!("Invalid callback status value: {}", value)), + } + } +} + +impl From for u8 { + fn from(status: RequestCallbackStatus) -> Self { + status as u8 + } } #[cfg(test)] pub mod mock { use { - crate::chain::reader::{BlockNumber, BlockStatus, EntropyReader, Request}, + crate::chain::reader::{ + BlockNumber, BlockStatus, EntropyReader, Request, RequestCallbackStatus, + }, anyhow::Result, axum::async_trait, ethers::types::{Address, U256}, @@ -132,6 +168,7 @@ pub mod mock { sequence_number: s, block_number: b, use_blockhash: u, + callback_status: RequestCallbackStatus::CallbackNotNecessary, }) .collect(), ), @@ -151,6 +188,7 @@ pub mod mock { sequence_number: sequence, block_number, use_blockhash, + callback_status: RequestCallbackStatus::CallbackNotNecessary, }); self } diff --git a/apps/fortuna/src/keeper/process_event.rs b/apps/fortuna/src/keeper/process_event.rs index 8604996d80..7a91253be7 100644 --- a/apps/fortuna/src/keeper/process_event.rs +++ b/apps/fortuna/src/keeper/process_event.rs @@ -1,7 +1,10 @@ use { super::keeper_metrics::AccountLabel, crate::{ - chain::{ethereum::PythRandomErrorsErrors, reader::RequestedWithCallbackEvent}, + chain::{ + ethereum::PythRandomErrorsErrors, + reader::{RequestCallbackStatus, RequestedWithCallbackEvent}, + }, eth_utils::utils::{submit_tx_with_backoff, SubmitTxError}, history::{RequestEntryState, RequestStatus}, keeper::block::ProcessParams, @@ -56,14 +59,20 @@ pub async fn process_event_with_backoff( // Check if the request is still open after the delay. // If it is, we will process it as a backup replica. - - // FIXME(Tejas): check callback status match chain_state .contract .get_request_v2(event.provider_address, event.sequence_number) .await { - Ok(Some(_)) => { + Ok(Some(req)) => { + // If the request is in the CallbackNotStarted state, it means that the primary replica + // has not yet called the callback. We should process it as a backup replica. + if req.callback_status != RequestCallbackStatus::CallbackNotStarted { + tracing::debug!( + "Request already handled by primary replica during delay, skipping" + ); + return Ok(()); + } tracing::info!( delay_seconds = replica_config.backup_delay_seconds, "Request still open after delay, processing as backup replica" @@ -71,7 +80,7 @@ pub async fn process_event_with_backoff( } Ok(None) => { tracing::debug!( - "Request already fulfilled by primary replica during delay, skipping" + "Request already handled by primary replica during delay, skipping" ); return Ok(()); } From b12e75aa123b7cc2f4e4530b1b7cd8d75ef33402 Mon Sep 17 00:00:00 2001 From: Tejas Badadare Date: Mon, 7 Jul 2025 14:20:25 -0700 Subject: [PATCH 7/7] use query() instead of query! macro --- apps/fortuna/.gitignore | 2 +- apps/fortuna/src/history.rs | 67 ++++++++++++++++++------------------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/apps/fortuna/.gitignore b/apps/fortuna/.gitignore index 9fb1d0163d..b978440e2e 100644 --- a/apps/fortuna/.gitignore +++ b/apps/fortuna/.gitignore @@ -3,4 +3,4 @@ *secret* *private-key* .envrc -fortuna.db +fortuna.db* diff --git a/apps/fortuna/src/history.rs b/apps/fortuna/src/history.rs index 03c5251fe8..245e5dea18 100644 --- a/apps/fortuna/src/history.rs +++ b/apps/fortuna/src/history.rs @@ -259,20 +259,19 @@ impl History { let block_number = new_status.request_block_number as i64; let sender: String = new_status.sender.encode_hex(); let user_random_number: String = new_status.user_random_number.encode_hex(); - sqlx::query!("INSERT INTO request(chain_id, network_id, provider, sequence, created_at, last_updated_at, state, request_block_number, request_tx_hash, user_random_number, sender, gas_limit) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", - chain_id, - network_id, - provider, - sequence, - new_status.created_at, - new_status.last_updated_at, - "Pending", - block_number, - request_tx_hash, - user_random_number, - sender, - gas_limit - ) + sqlx::query("INSERT INTO request(chain_id, network_id, provider, sequence, created_at, last_updated_at, state, request_block_number, request_tx_hash, user_random_number, sender, gas_limit) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)") + .bind(chain_id.clone()) + .bind(network_id) + .bind(provider.clone()) + .bind(sequence) + .bind(new_status.created_at) + .bind(new_status.last_updated_at) + .bind("Pending") + .bind(block_number) + .bind(request_tx_hash.clone()) + .bind(user_random_number) + .bind(sender) + .bind(gas_limit.clone()) .execute(pool) .await } @@ -287,17 +286,17 @@ impl History { let reveal_tx_hash: String = reveal_tx_hash.encode_hex(); let provider_random_number: String = provider_random_number.encode_hex(); let gas_used: String = gas_used.to_string(); - let result = sqlx::query!("UPDATE request SET state = ?, last_updated_at = ?, reveal_block_number = ?, reveal_tx_hash = ?, provider_random_number =?, gas_used = ? WHERE network_id = ? AND sequence = ? AND provider = ? AND request_tx_hash = ?", - "Completed", - new_status.last_updated_at, - reveal_block_number, - reveal_tx_hash, - provider_random_number, - gas_used, - network_id, - sequence, - provider, - request_tx_hash) + let result = sqlx::query("UPDATE request SET state = ?, last_updated_at = ?, reveal_block_number = ?, reveal_tx_hash = ?, provider_random_number =?, gas_used = ? WHERE network_id = ? AND sequence = ? AND provider = ? AND request_tx_hash = ?") + .bind("Completed") + .bind(new_status.last_updated_at) + .bind(reveal_block_number) + .bind(reveal_tx_hash) + .bind(provider_random_number) + .bind(gas_used) + .bind(network_id) + .bind(sequence) + .bind(provider.clone()) + .bind(request_tx_hash.clone()) .execute(pool) .await; if let Ok(query_result) = &result { @@ -313,15 +312,15 @@ impl History { } => { let provider_random_number: Option = provider_random_number .map(|provider_random_number| provider_random_number.encode_hex()); - sqlx::query!("UPDATE request SET state = ?, last_updated_at = ?, info = ?, provider_random_number = ? WHERE network_id = ? AND sequence = ? AND provider = ? AND request_tx_hash = ? AND state = 'Pending'", - "Failed", - new_status.last_updated_at, - reason, - provider_random_number, - network_id, - sequence, - provider, - request_tx_hash) + sqlx::query("UPDATE request SET state = ?, last_updated_at = ?, info = ?, provider_random_number = ? WHERE network_id = ? AND sequence = ? AND provider = ? AND request_tx_hash = ? AND state = 'Pending'") + .bind("Failed") + .bind(new_status.last_updated_at) + .bind(reason) + .bind(provider_random_number) + .bind(network_id) + .bind(sequence) + .bind(provider) + .bind(request_tx_hash) .execute(pool) .await }