diff --git a/CHANGELOG.md b/CHANGELOG.md index 30590d4e9..b15ae93e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## v0.5.4 (2024-10-11) +- Fixed gas estimation for when a tx has gas limit < block gas limit but with the L1 fee overhead the gas estimation is returned > block gas limit. Preventing transactions from landing on chain. ([#1323](https://github.com/chainwayxyz/citrea/pull/1323)) +- Better use of `tokio::spawn_blocking` in Bitcoin DA adapter. ([#1321](https://github.com/chainwayxyz/citrea/pull/1321) [#1324](https://github.com/chainwayxyz/citrea/pull/1324)) + ## v0.5.3 (2024-10-10) - `eth_call` RPC now supports state and block overrides. ([#1270](https://github.com/chainwayxyz/citrea/pull/1270)) - `eth_call`, `eth_estimateGas` and `eth_createAccessList` RPCs now supports "pending" block tag. ([#1303](https://github.com/chainwayxyz/citrea/pull/1303)) diff --git a/crates/bitcoin-da/src/helpers/builders/batch_proof_namespace.rs b/crates/bitcoin-da/src/helpers/builders/batch_proof_namespace.rs index 7aaa67a2c..c64f0627e 100644 --- a/crates/bitcoin-da/src/helpers/builders/batch_proof_namespace.rs +++ b/crates/bitcoin-da/src/helpers/builders/batch_proof_namespace.rs @@ -54,7 +54,7 @@ impl TxListWithReveal for BatchProvingTxs { #[instrument(level = "trace", skip_all, err)] pub fn create_seqcommitment_transactions( body: Vec, - da_private_key: &SecretKey, + da_private_key: SecretKey, prev_utxo: Option, utxos: Vec, change_address: Address, @@ -62,11 +62,11 @@ pub fn create_seqcommitment_transactions( commit_fee_rate: u64, reveal_fee_rate: u64, network: Network, - reveal_tx_prefix: &[u8], + reveal_tx_prefix: Vec, ) -> Result { create_batchproof_type_0( body, - da_private_key, + &da_private_key, prev_utxo, utxos, change_address, @@ -74,7 +74,7 @@ pub fn create_seqcommitment_transactions( commit_fee_rate, reveal_fee_rate, network, - reveal_tx_prefix, + &reveal_tx_prefix, ) } diff --git a/crates/bitcoin-da/src/helpers/builders/light_client_proof_namespace.rs b/crates/bitcoin-da/src/helpers/builders/light_client_proof_namespace.rs index b0686d843..26d543c04 100644 --- a/crates/bitcoin-da/src/helpers/builders/light_client_proof_namespace.rs +++ b/crates/bitcoin-da/src/helpers/builders/light_client_proof_namespace.rs @@ -87,7 +87,7 @@ impl TxListWithReveal for LightClientTxs { #[instrument(level = "trace", skip_all, err)] pub fn create_zkproof_transactions( body: Vec, - da_private_key: &SecretKey, + da_private_key: SecretKey, prev_utxo: Option, utxos: Vec, change_address: Address, @@ -95,12 +95,12 @@ pub fn create_zkproof_transactions( commit_fee_rate: u64, reveal_fee_rate: u64, network: Network, - reveal_tx_prefix: &[u8], + reveal_tx_prefix: Vec, ) -> Result { if body.len() < MAX_TXBODY_SIZE { create_inscription_type_0( body, - da_private_key, + &da_private_key, prev_utxo, utxos, change_address, @@ -108,12 +108,12 @@ pub fn create_zkproof_transactions( commit_fee_rate, reveal_fee_rate, network, - reveal_tx_prefix, + &reveal_tx_prefix, ) } else { create_inscription_type_1( body, - da_private_key, + &da_private_key, prev_utxo, utxos, change_address, @@ -121,7 +121,7 @@ pub fn create_zkproof_transactions( commit_fee_rate, reveal_fee_rate, network, - reveal_tx_prefix, + &reveal_tx_prefix, ) } } diff --git a/crates/bitcoin-da/src/helpers/builders/tests.rs b/crates/bitcoin-da/src/helpers/builders/tests.rs index dfefe66f3..950939ea9 100644 --- a/crates/bitcoin-da/src/helpers/builders/tests.rs +++ b/crates/bitcoin-da/src/helpers/builders/tests.rs @@ -452,6 +452,7 @@ fn build_reveal_transaction() { assert!(tx.is_err()); assert_eq!(format!("{}", tx.unwrap_err()), "input UTXO not big enough"); } + #[test] fn create_inscription_transactions() { let (body, address, utxos) = get_mock_data(); @@ -465,7 +466,7 @@ fn create_inscription_transactions() { let LightClientTxs::Complete { commit, reveal } = super::light_client_proof_namespace::create_zkproof_transactions( body.clone(), - &da_private_key, + da_private_key, None, utxos.clone(), address.clone(), @@ -473,7 +474,7 @@ fn create_inscription_transactions() { 12, 10, bitcoin::Network::Bitcoin, - tx_prefix, + tx_prefix.to_vec(), ) .unwrap() else { diff --git a/crates/bitcoin-da/src/service.rs b/crates/bitcoin-da/src/service.rs index 03286f324..e9a13efd0 100644 --- a/crates/bitcoin-da/src/service.rs +++ b/crates/bitcoin-da/src/service.rs @@ -183,92 +183,85 @@ impl BitcoinService { self: Arc, mut rx: UnboundedReceiver>>, ) { - // This should be spawn_blocking, since it is a CPU-bound worker. - // When spawned with tokio::spawn, it blocks other futures and - // disrupts tokio runtime. - tokio::task::spawn_blocking(|| { - tokio::runtime::Handle::current().block_on(async move { - let mut prev_utxo = match self.get_prev_utxo().await { - Ok(Some(prev_utxo)) => Some(prev_utxo), - Ok(None) => { - info!("No pending transactions found"); - None - } - Err(e) => { - error!(?e, "Failed to get pending transactions"); - None - } - }; + tokio::spawn(async move { + let mut prev_utxo = match self.get_prev_utxo().await { + Ok(Some(prev_utxo)) => Some(prev_utxo), + Ok(None) => { + info!("No pending transactions found"); + None + } + Err(e) => { + error!(?e, "Failed to get pending transactions"); + None + } + }; - trace!("BitcoinDA queue is initialized. Waiting for the first request..."); - - loop { - select! { - request_opt = rx.recv() => { - if let Some(request_opt) = request_opt { - match request_opt { - Some(request) => { - trace!("A new request is received"); - let prev = prev_utxo.take(); - loop { - // Build and send tx with retries: - let fee_sat_per_vbyte = match self.get_fee_rate().await { - Ok(rate) => rate, - Err(e) => { - error!(?e, "Failed to call get_fee_rate. Retrying..."); - tokio::time::sleep(Duration::from_secs(1)).await; - continue; - } - }; - match self - .send_transaction_with_fee_rate( - prev.clone(), - request.da_data.clone(), - fee_sat_per_vbyte, - ) - .await - { - Ok(tx) => { - let tx_id = TxidWrapper(tx.id); - info!(%tx.id, "Sent tx to BitcoinDA"); - prev_utxo = Some(UTXO { - tx_id: tx.id, - vout: 0, - script_pubkey: tx.tx.output[0].script_pubkey.to_hex_string(), - address: None, - amount: tx.tx.output[0].value.to_sat(), - confirmations: 0, - spendable: true, - solvable: true, - }); - - let _ = request.notify.send(Ok(tx_id)); - } - Err(e) => { - error!(?e, "Failed to send transaction to DA layer"); - tokio::time::sleep(Duration::from_secs(1)).await; - continue; - } + trace!("BitcoinDA queue is initialized. Waiting for the first request..."); + + loop { + select! { + request_opt = rx.recv() => { + if let Some(request_opt) = request_opt { + match request_opt { + Some(request) => { + trace!("A new request is received"); + let prev = prev_utxo.take(); + loop { + // Build and send tx with retries: + let fee_sat_per_vbyte = match self.get_fee_rate().await { + Ok(rate) => rate, + Err(e) => { + error!(?e, "Failed to call get_fee_rate. Retrying..."); + tokio::time::sleep(Duration::from_secs(1)).await; + continue; + } + }; + match self + .send_transaction_with_fee_rate( + prev.clone(), + request.da_data.clone(), + fee_sat_per_vbyte, + ) + .await + { + Ok(tx) => { + let tx_id = TxidWrapper(tx.id); + info!(%tx.id, "Sent tx to BitcoinDA"); + prev_utxo = Some(UTXO { + tx_id: tx.id, + vout: 0, + script_pubkey: tx.tx.output[0].script_pubkey.to_hex_string(), + address: None, + amount: tx.tx.output[0].value.to_sat(), + confirmations: 0, + spendable: true, + solvable: true, + }); + + let _ = request.notify.send(Ok(tx_id)); + } + Err(e) => { + error!(?e, "Failed to send transaction to DA layer"); + tokio::time::sleep(Duration::from_secs(1)).await; + continue; } - break; } - } - - None => { - info!("Shutdown signal received. Stopping BitcoinDA queue."); break; } } + + None => { + info!("Shutdown signal received. Stopping BitcoinDA queue."); + break; + } } - }, - _ = signal::ctrl_c() => { - return; } + }, + _ = signal::ctrl_c() => { + return; } } - }); - - error!("BitcoinDA queue stopped"); + } }); } @@ -374,19 +367,26 @@ impl BitcoinService { let data = DaDataLightClient::ZKProof(zkproof); let blob = borsh::to_vec(&data).expect("DaDataLightClient serialize must not fail"); let blob = compress_blob(&blob); + + let reveal_light_client_prefix = self.reveal_light_client_prefix.clone(); // create inscribe transactions - let inscription_txs = create_zkproof_transactions( - blob, - &da_private_key, - prev_utxo, - utxos, - address, - REVEAL_OUTPUT_AMOUNT, - fee_sat_per_vbyte, - fee_sat_per_vbyte, - network, - &self.reveal_light_client_prefix, - )?; + let inscription_txs = tokio::task::spawn_blocking(move || { + // Since this is CPU bound work, we use spawn_blocking + // to release the tokio runtime execution + create_zkproof_transactions( + blob, + da_private_key, + prev_utxo, + utxos, + address, + REVEAL_OUTPUT_AMOUNT, + fee_sat_per_vbyte, + fee_sat_per_vbyte, + network, + reveal_light_client_prefix, + ) + }) + .await??; // write txs to file, it can be used to continue revealing blob if something goes wrong inscription_txs.write_to_file(self.tx_backup_dir.clone())?; @@ -409,19 +409,26 @@ impl BitcoinService { DaData::SequencerCommitment(comm) => { let data = DaDataBatchProof::SequencerCommitment(comm); let blob = borsh::to_vec(&data).expect("DaDataBatchProof serialize must not fail"); + + let reveal_batch_prover_prefix = self.reveal_batch_prover_prefix.clone(); // create inscribe transactions - let inscription_txs = create_seqcommitment_transactions( - blob, - &da_private_key, - prev_utxo, - utxos, - address, - REVEAL_OUTPUT_AMOUNT, - fee_sat_per_vbyte, - fee_sat_per_vbyte, - network, - &self.reveal_batch_prover_prefix, - )?; + let inscription_txs = tokio::task::spawn_blocking(move || { + // Since this is CPU bound work, we use spawn_blocking + // to release the tokio runtime execution + create_seqcommitment_transactions( + blob, + da_private_key, + prev_utxo, + utxos, + address, + REVEAL_OUTPUT_AMOUNT, + fee_sat_per_vbyte, + fee_sat_per_vbyte, + network, + reveal_batch_prover_prefix, + ) + }) + .await??; // write txs to file, it can be used to continue revealing blob if something goes wrong inscription_txs.write_to_file(self.tx_backup_dir.clone())?; diff --git a/crates/evm/src/query.rs b/crates/evm/src/query.rs index 767f17d77..3739f1d34 100644 --- a/crates/evm/src/query.rs +++ b/crates/evm/src/query.rs @@ -57,7 +57,7 @@ const ESTIMATE_GAS_ERROR_RATIO: f64 = 0.015; #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub(crate) struct EstimatedTxExpenses { /// Evm gas used. - gas_used: U64, + pub gas_used: U64, /// Base fee of the L2 block when tx was executed. base_fee: U256, /// L1 fee. @@ -684,7 +684,7 @@ impl Evm { Ok(AccessListWithGasUsed { access_list, - gas_used: estimated.gas_with_l1_overhead(), + gas_used: gas_limit_to_return(U64::from(block_env.gas_limit), estimated), }) } @@ -735,7 +735,17 @@ impl Evm { working_set: &mut WorkingSet, ) -> RpcResult { let estimated = self.estimate_tx_expenses(request, block_number, working_set)?; - Ok(estimated.gas_with_l1_overhead()) + + // TODO: this assumes all blocks have the same gas limit + // if gas limit ever changes this should be updated + let last_block = self + .blocks + .last(&mut working_set.accessory_state()) + .expect("Head block must be set"); + + let block_gas_limit = U64::from(last_block.header.gas_limit); + + Ok(gas_limit_to_return(block_gas_limit, estimated)) } /// Handler for: `eth_estimateDiffSize` @@ -1706,6 +1716,30 @@ fn set_state_to_end_of_evm_block( working_set.set_archival_version(block_number + 1); } +/// We add some kind of L1 fee overhead to the estimated gas +/// so that the receiver can check if their balance is enough to +/// cover L1 fees, without ever knowing about L1 fees. +/// +/// However, there is a chance that the real gas used is not bigger than +/// block gas limit, but it is bigger with the overhead added. In this +/// case the mempool will reject the transaction and even if it didn't, the sequencer +/// wouldn't put it into a block since it's against EVM rules to hava tx that +/// has more gas limit than the block. +/// +/// But in the case where the gas used is already bigger than the block gas limit +/// we want to return the gas estimation as is since the mempool will reject it +/// anyway. +#[inline] +fn gas_limit_to_return(block_gas_limit: U64, estimated_tx_expenses: EstimatedTxExpenses) -> U256 { + if estimated_tx_expenses.gas_used > block_gas_limit { + estimated_tx_expenses.gas_with_l1_overhead() + } else { + let with_l1_overhead = estimated_tx_expenses.gas_with_l1_overhead(); + + with_l1_overhead.min(U256::from(block_gas_limit)) + } +} + /// Creates the next blocks `BlockEnv` based on the latest block /// Also updates `Evm::latest_block_hashes` with the new block hash fn get_pending_block_env( @@ -1745,3 +1779,32 @@ fn get_pending_block_env( block_env } + +#[test] +fn test_gas_limit_to_return() { + assert_eq!( + gas_limit_to_return( + U64::from(8_000_000), + EstimatedTxExpenses { + gas_used: U64::from(5_000_000), + base_fee: U256::from(10000000), // 0.01 gwei + l1_fee: U256::from(40_000_000_000_000u128), + l1_diff_size: 1 // not relevant to this test + } + ), + U256::from(8_000_000) + ); + + assert_eq!( + gas_limit_to_return( + U64::from(8_000_000), + EstimatedTxExpenses { + gas_used: U64::from(8_000_001), + base_fee: U256::from(10000000), // 0.01 gwei + l1_fee: U256::from(40_000_000u128), + l1_diff_size: 1 // not relevant to this test + } + ), + U256::from(8_000_005) + ); +} diff --git a/crates/sequencer/src/mempool.rs b/crates/sequencer/src/mempool.rs index e477fdd2b..68903e03c 100644 --- a/crates/sequencer/src/mempool.rs +++ b/crates/sequencer/src/mempool.rs @@ -85,6 +85,9 @@ impl CitreaMempool { let validator = TransactionValidationTaskExecutor::eth_builder(Arc::new(chain_spec)) .no_cancun() .no_eip4844() + // TODO: if we ever increase block gas limits, we need to pull this from + // somewhere else + .set_block_gas_limit(evm_config.block_gas_limit) .set_shanghai(true) .with_additional_tasks(0) .build_with_tasks(client, TokioTaskExecutor::default(), blob_store); diff --git a/crates/sovereign-sdk/rollup-interface/src/lib.rs b/crates/sovereign-sdk/rollup-interface/src/lib.rs index e06d79054..c11114493 100644 --- a/crates/sovereign-sdk/rollup-interface/src/lib.rs +++ b/crates/sovereign-sdk/rollup-interface/src/lib.rs @@ -12,7 +12,7 @@ extern crate alloc; /// /// Mostly used for web3_clientVersion RPC calls and might be used for other purposes. #[cfg(feature = "native")] -pub const CITREA_VERSION: &str = env!("CARGO_PKG_VERSION"); +pub const CITREA_VERSION: &str = "v0.5.4"; mod state_machine; pub use state_machine::*; diff --git a/docs/run-testnet.md b/docs/run-testnet.md index 10d37abad..5339cd63b 100644 --- a/docs/run-testnet.md +++ b/docs/run-testnet.md @@ -117,12 +117,12 @@ Finally run this command to run your Citrea full node: Mac: ```sh -./citrea-v0.5.3-osx-arm64 --da-layer bitcoin --rollup-config-path ./rollup_config.toml --genesis-paths ./genesis +./citrea-v0.5.4-osx-arm64 --da-layer bitcoin --rollup-config-path ./rollup_config.toml --genesis-paths ./genesis ``` Linux: ```sh -./citrea-v0.5.3-linux-amd64 --da-layer bitcoin --rollup-config-path ./rollup_config.toml --genesis-paths ./genesis +./citrea-v0.5.4-linux-amd64 --da-layer bitcoin --rollup-config-path ./rollup_config.toml --genesis-paths ./genesis ``` Your full node should be serving RPC at `http://0.0.0.0:8080` now.