Skip to content

feat(fortuna): Dynamically increase gas limit for a transaction on failure #2238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 10, 2025
Merged

feat(fortuna): Dynamically increase gas limit for a transaction on failure #2238

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc4fee0
gas multiplier for tx backoff
jayantk Jan 10, 2025
e019de8
add scaling to callback
jayantk Jan 10, 2025
a894665
bump version
jayantk Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/fortuna/config.sample.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ chains:
# Keeper configuration for the chain
reveal_delay_blocks: 0
gas_limit: 500000
# Increase the transaction gas limit by 10% each time the callback fails
backoff_gas_multiplier_pct: 110
min_keeper_balance: 100000000000000000

# Provider configuration
Expand Down
8 changes: 8 additions & 0 deletions apps/fortuna/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ pub struct EthereumConfig {
/// The gas limit to use for entropy callback transactions.
pub gas_limit: u64,

/// The percentage multiplier to apply to the gas limit for each backoff.
#[serde(default = "default_backoff_gas_multiplier_pct")]
pub backoff_gas_multiplier_pct: u64,

/// The minimum percentage profit to earn as a function of the callback cost.
/// For example, 20 means a profit of 20% over the cost of the callback.
/// The fee will be raised if the profit is less than this number.
Expand Down Expand Up @@ -172,6 +176,10 @@ pub struct EthereumConfig {
pub priority_fee_multiplier_pct: u64,
}

fn default_backoff_gas_multiplier_pct() -> u64 {
100
Copy link
Contributor Author

@jayantk jayantk Jan 10, 2025

Choose a reason for hiding this comment

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

we may want to set this to some reasonable % in the future, but let's see how the abstract thing goes first.

}

/// A commitment that the provider used to generate random numbers at some point in the past.
/// These historical commitments need to be stored in the configuration to support transition points where
/// the commitment changes. In theory, this information is stored on the blockchain, but unfortunately it
Expand Down
53 changes: 48 additions & 5 deletions apps/fortuna/src/keeper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ 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;
/// By default, we scale the gas estimate by 25% when submitting the tx.
const DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT: u64 = 125;

#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)]
pub struct AccountLabel {
Expand Down Expand Up @@ -254,6 +256,7 @@ pub async fn run_keeper_threads(
},
contract.clone(),
gas_limit,
chain_eth_config.backoff_gas_multiplier_pct,
chain_state.clone(),
metrics.clone(),
fulfilled_requests_cache.clone(),
Expand All @@ -279,6 +282,7 @@ pub async fn run_keeper_threads(
rx,
Arc::clone(&contract),
gas_limit,
chain_eth_config.backoff_gas_multiplier_pct,
metrics.clone(),
fulfilled_requests_cache.clone(),
)
Expand All @@ -303,7 +307,9 @@ pub async fn run_keeper_threads(
chain_state.provider_address,
ADJUST_FEE_INTERVAL,
chain_eth_config.legacy_tx,
chain_eth_config.gas_limit,
// NOTE: we adjust fees based on the maximum gas that the keeper will submit a callback with.
// This number is *larger* than the configured gas limit, as we pad gas on transaction submission for reliability.
(chain_eth_config.gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100,
Copy link
Contributor Author

@jayantk jayantk Jan 10, 2025

Choose a reason for hiding this comment

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

We were previously multiplying by 5/4 in the function that sends the TX, but we weren't accounting for that when setting the tx fee.

chain_eth_config.min_profit_pct,
chain_eth_config.target_profit_pct,
chain_eth_config.max_profit_pct,
Expand Down Expand Up @@ -372,6 +378,7 @@ pub async fn process_event_with_backoff(
chain_state: BlockchainState,
contract: Arc<InstrumentedSignablePythContract>,
gas_limit: U256,
backoff_gas_multiplier_pct: u64,
metrics: Arc<KeeperMetrics>,
) {
let start_time = std::time::Instant::now();
Expand All @@ -388,13 +395,35 @@ pub async fn process_event_with_backoff(
max_elapsed_time: Some(Duration::from_secs(300)), // retry for 5 minutes
..Default::default()
};

let current_multiplier = Arc::new(AtomicU64::new(DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT));

match backoff::future::retry_notify(
backoff,
|| async {
process_event(&event, &chain_state, &contract, gas_limit, metrics.clone()).await
let multiplier = current_multiplier.load(std::sync::atomic::Ordering::Relaxed);
process_event(
&event,
&chain_state,
&contract,
gas_limit,
multiplier,
metrics.clone(),
)
.await
},
|e, dur| {
tracing::error!("Error happened at {:?}: {}", dur, e);
let multiplier = current_multiplier.load(std::sync::atomic::Ordering::Relaxed);
tracing::error!(
"Error at duration {:?} with gas multiplier {}: {}",
dur,
multiplier,
e
);
current_multiplier.store(
multiplier.saturating_mul(backoff_gas_multiplier_pct) / 100,
std::sync::atomic::Ordering::Relaxed,
);
},
)
.await
Expand Down Expand Up @@ -436,6 +465,8 @@ pub async fn process_event(
chain_config: &BlockchainState,
contract: &InstrumentedSignablePythContract,
gas_limit: U256,
// A value of 100 submits the tx with the same gas as the estimate.
gas_estimate_multiplier_pct: u64,
metrics: Arc<KeeperMetrics>,
) -> Result<(), backoff::Error<anyhow::Error>> {
// ignore requests that are not for the configured provider
Expand Down Expand Up @@ -466,6 +497,8 @@ pub async fn process_event(
backoff::Error::transient(anyhow!("Error estimating gas for reveal: {:?}", e))
})?;

// The gas limit on the simulated transaction is the configured gas limit on the chain,
// 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 {} > {}",
Expand All @@ -474,8 +507,10 @@ pub async fn process_event(
)));
}

// Pad the gas estimate by 25% after checking it against the gas limit
let gas_estimate = gas_estimate.saturating_mul(5.into()) / 4;
// Pad the gas estimate after checking it against the simulation gas limit, ensuring that
// the padded gas estimate doesn't exceed the maximum amount of gas we are willing to use.
let gas_estimate = gas_estimate.saturating_mul(gas_estimate_multiplier_pct.into()) / 100;
let gas_estimate = gas_estimate.min((gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100);

let contract_call = contract
.reveal_with_callback(
Expand Down Expand Up @@ -589,6 +624,7 @@ pub async fn process_block_range(
block_range: BlockRange,
contract: Arc<InstrumentedSignablePythContract>,
gas_limit: U256,
backoff_gas_multiplier_pct: u64,
chain_state: api::BlockchainState,
metrics: Arc<KeeperMetrics>,
fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
Expand All @@ -612,6 +648,7 @@ pub async fn process_block_range(
},
contract.clone(),
gas_limit,
backoff_gas_multiplier_pct,
chain_state.clone(),
metrics.clone(),
fulfilled_requests_cache.clone(),
Expand All @@ -634,6 +671,7 @@ pub async fn process_single_block_batch(
block_range: BlockRange,
contract: Arc<InstrumentedSignablePythContract>,
gas_limit: U256,
backoff_gas_multiplier_pct: u64,
chain_state: api::BlockchainState,
metrics: Arc<KeeperMetrics>,
fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
Expand All @@ -660,6 +698,7 @@ pub async fn process_single_block_batch(
chain_state.clone(),
contract.clone(),
gas_limit,
backoff_gas_multiplier_pct,
metrics.clone(),
)
.in_current_span(),
Expand Down Expand Up @@ -806,6 +845,7 @@ pub async fn process_new_blocks(
mut rx: mpsc::Receiver<BlockRange>,
contract: Arc<InstrumentedSignablePythContract>,
gas_limit: U256,
backoff_gas_multiplier_pct: u64,
metrics: Arc<KeeperMetrics>,
fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
) {
Expand All @@ -816,6 +856,7 @@ pub async fn process_new_blocks(
block_range,
Arc::clone(&contract),
gas_limit,
backoff_gas_multiplier_pct,
chain_state.clone(),
metrics.clone(),
fulfilled_requests_cache.clone(),
Expand All @@ -832,6 +873,7 @@ pub async fn process_backlog(
backlog_range: BlockRange,
contract: Arc<InstrumentedSignablePythContract>,
gas_limit: U256,
backoff_gas_multiplier_pct: u64,
chain_state: BlockchainState,
metrics: Arc<KeeperMetrics>,
fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
Expand All @@ -841,6 +883,7 @@ pub async fn process_backlog(
backlog_range,
contract,
gas_limit,
backoff_gas_multiplier_pct,
chain_state,
metrics,
fulfilled_requests_cache,
Expand Down
Loading