diff --git a/crates/client/src/btc_chain/esplora_bitcoin_adaptor.rs b/crates/client/src/btc_chain/esplora_bitcoin_adaptor.rs index a3785a0d7..0319940e9 100644 --- a/crates/client/src/btc_chain/esplora_bitcoin_adaptor.rs +++ b/crates/client/src/btc_chain/esplora_bitcoin_adaptor.rs @@ -8,8 +8,15 @@ use std::future::Future; use std::time::Duration; use tracing::warn; -const TEST_URL: &str = "https://mempool.space/testnet/api"; const MAIN_URL: &str = "https://mempool.space/api"; +const TESTNET4_URL: &str = "https://mempool.space/testnet4/api"; +const SIGNET_URL: &str = "https://mempool.space/signet/api"; +const TESTNET3_URL: &str = "https://mempool.space/testnet/api"; + +/// Retries esplora-client makes for 429 and 5xx answers. Its backoff starts at 256 ms +/// and doubles, so three retries wait about 1.8 s in all and fit in the request +/// timeout; its default of six waits about 16 s, longer than the default timeout. +const ESPLORA_MAX_RETRIES: usize = 3; #[derive(Debug)] pub struct BtcRpcTimeoutError { @@ -32,7 +39,10 @@ impl std::error::Error for BtcRpcTimeoutError {} pub fn get_esplora_url(network: Network) -> &'static str { match network { Network::Bitcoin => MAIN_URL, - _ => TEST_URL, + Network::Testnet4 => TESTNET4_URL, + Network::Signet => SIGNET_URL, + // testnet3; regtest has no public explorer and needs an explicit URL + _ => TESTNET3_URL, } } @@ -47,6 +57,8 @@ impl EsploraBitcoinAdaptor { let timeout_secs = get_btc_request_timeout_secs(); EsploraBitcoinAdaptor { esplora: Builder::new(esplora_url.unwrap_or(get_esplora_url(network))) + .timeout(timeout_secs) + .max_retries(ESPLORA_MAX_RETRIES) .build_async() .expect("Could not build esplora client"), network, @@ -142,3 +154,16 @@ impl BitcoinAdaptor for EsploraBitcoinAdaptor { self.with_timeout("get_header_by_hash", self.esplora.get_header_by_hash(block_hash)).await } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_explorer_matches_the_network() { + assert_eq!(get_esplora_url(Network::Bitcoin), MAIN_URL); + assert_eq!(get_esplora_url(Network::Testnet4), TESTNET4_URL); + assert_eq!(get_esplora_url(Network::Signet), SIGNET_URL); + assert_eq!(get_esplora_url(Network::Testnet), TESTNET3_URL); + } +} diff --git a/crates/client/src/btc_chain/mempool_v1_type.rs b/crates/client/src/btc_chain/mempool_v1_type.rs deleted file mode 100644 index fbdfd3065..000000000 --- a/crates/client/src/btc_chain/mempool_v1_type.rs +++ /dev/null @@ -1,132 +0,0 @@ -use crate::btc_chain::esplora_bitcoin_adaptor::get_esplora_url; -use bitcoin::{BlockHash, Network}; -use serde::{Deserialize, Serialize}; - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct V1Block { - pub height: u64, - pub timestamp: u64, - pub tx_count: u64, - pub size: u64, - pub extras: V1BlockExtras, - // pub id: String, - // pub version: u64, - // pub bits: u64, - // pub nonce: u64, - // pub difficulty: f64, - // pub merkle_root: String, - // pub weight: u64, - // pub previousblockhash: Option, - // pub mediantime: u64, - // #[serde(default)] - // pub stale: Option, -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct V1BlockExtras { - #[serde(rename = "totalFees")] - pub total_fees: u64, - #[serde(rename = "medianFee")] - pub median_fee: u64, - #[serde(rename = "feeRange")] - pub fee_range: Vec, - // pub reward: u64, - // pub pool: V1PoolInfo, - // #[serde(rename = "avgFee")] - // pub avg_fee: u64, - // #[serde(rename = "avgFeeRate")] - // pub avg_fee_rate: u64, - // #[serde(rename = "coinbaseRaw")] - // pub coinbase_raw: String, - // #[serde(rename = "coinbaseAddress")] - // pub coinbase_address: Option, - // #[serde(rename = "coinbaseAddresses")] - // pub coinbase_addresses: Vec, - // #[serde(rename = "coinbaseSignature")] - // pub coinbase_signature: String, - // #[serde(rename = "coinbaseSignatureAscii")] - // pub coinbase_signature_ascii: String, - // #[serde(rename = "avgTxSize")] - // pub avg_tx_size: f64, - // #[serde(rename = "totalInputs")] - // pub total_inputs: u64, - // #[serde(rename = "totalOutputs")] - // pub total_outputs: u64, - // #[serde(rename = "totalOutputAmt")] - // pub total_output_amt: u64, - // #[serde(rename = "medianFeeAmt", default)] - // pub median_fee_amt: Option, - // #[serde(rename = "feePercentiles", default)] - // pub fee_percentiles: Option>, - // #[serde(rename = "segwitTotalTxs")] - // pub segwit_total_txs: u64, - // #[serde(rename = "segwitTotalSize")] - // pub segwit_total_size: u64, - // #[serde(rename = "segwitTotalWeight")] - // pub segwit_total_weight: u64, - // pub header: String, - // #[serde(rename = "utxoSetChange")] - // pub utxo_set_change: u64, - // #[serde(rename = "utxoSetSize")] - // pub utxo_set_size: Option, - // #[serde(rename = "totalInputAmt")] - // pub total_input_amt: Option, - // #[serde(rename = "virtualSize")] - // pub virtual_size: f64, - // #[serde(rename = "firstSeen")] - // pub first_seen: Option, - // pub orphans: Vec, - // #[serde(rename = "matchRate")] - // pub match_rate: Option, - // #[serde(rename = "expectedFees")] - // pub expected_fees: Option, - // #[serde(rename = "expectedWeight")] - // pub expected_weight: Option, - // #[serde(default)] - // pub similarity: Option, -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct V1PoolInfo { - pub id: u64, - pub name: String, - pub slug: String, - #[serde(rename = "minerNames")] - pub miner_names: Option>, -} - -pub type V1Blocks = Vec; - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct MempoolBlock { - #[serde(rename = "blockSize")] - pub block_size: u64, - #[serde(rename = "blockVSize")] - pub block_v_size: f64, - #[serde(rename = "nTx")] - pub n_tx: u64, - #[serde(rename = "medianFee")] - pub median_fee: u64, - #[serde(rename = "feeRange")] - pub fee_range: Vec, - #[serde(rename = "totalFees")] - pub total_fees: u64, -} - -pub type MempoolBlocks = Vec; - -pub fn get_v1_block_url(network: Network, block_hash: BlockHash) -> String { - let base_url = get_esplora_url(network); - format!("{base_url}/v1/block/{block_hash}") -} - -pub fn get_v1_blocks_url(network: Network, block_height: Option) -> String { - let base_url = get_esplora_url(network); - let path = - block_height.map(|h| format!("/v1/blocks/{h}")).unwrap_or_else(|| "/v1/blocks".to_string()); - format!("{base_url}{path}") -} - -pub fn get_v1_mempool_blocks_url(network: Network) -> String { - format!("{}/v1/fees/mempool-blocks", get_esplora_url(network)) -} diff --git a/crates/client/src/btc_chain/mod.rs b/crates/client/src/btc_chain/mod.rs index fc452836f..f84403ccd 100644 --- a/crates/client/src/btc_chain/mod.rs +++ b/crates/client/src/btc_chain/mod.rs @@ -10,7 +10,6 @@ pub mod bitcoin_adaptor; pub mod bitcoin_chain; mod esplora_bitcoin_adaptor; pub use esplora_bitcoin_adaptor::BtcRpcTimeoutError; -pub mod mempool_v1_type; mod mock_bitcoin_adaptor; #[derive(Debug)] diff --git a/crates/client/src/goat_chain/goat_adaptor.rs b/crates/client/src/goat_chain/goat_adaptor.rs index 6ffce2de7..f567ed13b 100644 --- a/crates/client/src/goat_chain/goat_adaptor.rs +++ b/crates/client/src/goat_chain/goat_adaptor.rs @@ -29,7 +29,9 @@ use alloy::{ }; use anyhow::{bail, format_err}; use async_trait::async_trait; +use std::collections::HashMap; use std::str::FromStr; +use std::sync::{Arc, LazyLock, Mutex}; use std::time::Duration; use tokio::time; use uuid::Uuid; @@ -469,6 +471,23 @@ type StakeManagementInstance = IStakeManagementInstance< >, >; +/// How long a send waits for its receipt before reporting failure. +const RECEIPT_WAIT_SECS: u64 = 30; + +/// One lock per sender address, shared by every adaptor in the process. It is held +/// from reading the nonce until the receipt arrives or the wait times out, so two +/// clients with the same key never sign with the same nonce. The nonce comes from +/// the latest block on purpose: with sends serialized it equals the pending nonce, +/// except after a timeout, where reusing it replaces the stuck transaction instead +/// of queueing a duplicate behind it. +static SENDER_LOCKS: LazyLock>>>> = + LazyLock::new(Default::default); + +fn sender_lock(sender: Address) -> Arc> { + let mut locks = SENDER_LOCKS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + locks.entry(sender).or_default().clone() +} + pub struct GoatAdaptor { chain_id: ChainId, signer: EthereumWallet, @@ -542,7 +561,8 @@ impl GoatAdaptor { &self, tx_request: TransactionRequest, ) -> anyhow::Result { - let (tx_hash, _) = self.handle_transaction_request_with_wait(tx_request, 10).await?; + let (tx_hash, _) = + self.handle_transaction_request_with_wait(tx_request, RECEIPT_WAIT_SECS).await?; Ok(tx_hash) } @@ -550,8 +570,9 @@ impl GoatAdaptor { &self, tx_request: TransactionRequest, ) -> anyhow::Result { - let (tx_hash, _) = - self.handle_transaction_request_with_wait_eip1559(tx_request, 10).await?; + let (tx_hash, _) = self + .handle_transaction_request_with_wait_eip1559(tx_request, RECEIPT_WAIT_SECS) + .await?; Ok(tx_hash) } @@ -583,11 +604,13 @@ impl GoatAdaptor { mut tx_request: TransactionRequest, max_wait_secs: u64, ) -> anyhow::Result<(TxHash, TransactionReceipt)> { + let sender = tx_request.from.ok_or_else(|| format_err!("transaction has no sender"))?; + let sender_lock = sender_lock(sender); + let _sending = sender_lock.lock().await; // update gas price nonce gas_limit tx_request.gas_price = Some(self.provider.clone().get_gas_price().await?); tracing::info!("gas price: {}", tx_request.gas_price.unwrap()); - tx_request.nonce = - Some(self.provider.clone().get_transaction_count(tx_request.from.unwrap()).await?); + tx_request.nonce = Some(self.provider.clone().get_transaction_count(sender).await?); tracing::info!("tx: {:?}", tx_request); tx_request.gas = Some(self.provider.clone().estimate_gas(tx_request.clone()).await?); tracing::info!("estimated gas: {:?}", tx_request.gas); @@ -639,7 +662,10 @@ impl GoatAdaptor { } }; } - bail!("tx_hash:{tx_hash} receipt not found within {max_wait_secs} seconds") + bail!( + "tx_hash:{tx_hash} receipt not found within {max_wait_secs} seconds; it may still be \ + pending, and the next transaction from this sender reuses its nonce and replaces it" + ) } async fn handle_transaction_request_with_wait_eip1559( @@ -647,6 +673,9 @@ impl GoatAdaptor { mut tx_request: TransactionRequest, max_wait_secs: u64, ) -> anyhow::Result<(TxHash, TransactionReceipt)> { + let sender = tx_request.from.ok_or_else(|| format_err!("transaction has no sender"))?; + let sender_lock = sender_lock(sender); + let _sending = sender_lock.lock().await; let (max_fee_per_gas, max_priority_fee_per_gas) = self.get_eip1559_fee_params().await?; tx_request.gas_price = None; tx_request.max_fee_per_gas = Some(max_fee_per_gas); @@ -656,8 +685,7 @@ impl GoatAdaptor { max_fee_per_gas, max_priority_fee_per_gas ); - tx_request.nonce = - Some(self.provider.clone().get_transaction_count(tx_request.from.unwrap()).await?); + tx_request.nonce = Some(self.provider.clone().get_transaction_count(sender).await?); tracing::info!("tx(type2): {:?}", tx_request); tx_request.gas = Some(self.provider.clone().estimate_gas(tx_request.clone()).await?); tracing::info!("estimated gas(type2): {:?}", tx_request.gas); @@ -706,7 +734,10 @@ impl GoatAdaptor { } }; } - bail!("tx_hash:{tx_hash} receipt not found within {max_wait_secs} seconds") + bail!( + "tx_hash:{tx_hash} receipt not found within {max_wait_secs} seconds; it may still be \ + pending, and the next transaction from this sender reuses its nonce and replaces it" + ) } } @@ -1726,6 +1757,22 @@ impl GoatAdaptor { } } +#[cfg(test)] +mod sender_lock_tests { + use super::*; + + #[test] + fn adaptors_share_one_lock_per_sender() { + let alice = Address::repeat_byte(0xa1); + let bob = Address::repeat_byte(0xb0); + let first = sender_lock(alice); + let _sending = first.try_lock().unwrap(); + // another adaptor with the same key waits; another key does not + assert!(sender_lock(alice).try_lock().is_err()); + assert!(sender_lock(bob).try_lock().is_ok()); + } +} + #[cfg(test)] mod digest_tests { use super::*; diff --git a/crates/client/src/goat_chain/mod.rs b/crates/client/src/goat_chain/mod.rs index 25fc01e97..6f48f85c3 100644 --- a/crates/client/src/goat_chain/mod.rs +++ b/crates/client/src/goat_chain/mod.rs @@ -8,7 +8,6 @@ use alloy::rpc::types::{ use anyhow::bail; use bitcoin::hashes::Hash; use bitcoin::{PublicKey, Transaction, Txid, XOnlyPublicKey}; -use tracing::info; use uuid::Uuid; pub mod utils; use crate::btc_chain::{BTCClient, MerkleProofExtend}; @@ -453,22 +452,13 @@ impl GOATClient { hex::encode(tx_proof_data.block_hash), hex::encode(block_hash_online) ); - if is_enable_update_spv_contract() { - // todo remove me later - info!( - "Btc chain rollback, update_spv_contract for height:{}", - tx_proof_data.height - ); - self.chain_service - .btc_spv_post_block_hash(tx_proof_data.height, &tx_proof_data.block_hash) - .await?; - } else { - bail!( - "instance_id:{instance_id} block_hash mismatch, from chain:{}, in contract:{}", - hex::encode(tx_proof_data.block_hash), - hex::encode(block_hash_online) - ); - } + // The SPV contract is written only by the relayer, which posts + // confirmed blocks; never overwrite it from this node's esplora view. + bail!( + "instance_id:{instance_id} block_hash mismatch, from chain:{}, in contract:{}", + hex::encode(tx_proof_data.block_hash), + hex::encode(block_hash_online) + ); } let pegin_amount_sats = tx.output[0].value.to_sat(); let min_pegin_fee_sats = self.gateway_get_min_pegin_fee_sats().await?; @@ -605,22 +595,13 @@ impl GOATClient { hex::encode(block_hash_online) ); - if is_enable_update_spv_contract() { - // todo remove me later - info!( - "Btc chain rollback, need to update_spv_contract for height:{}", - tx_proof_data.height - ); - self.chain_service - .btc_spv_post_block_hash(tx_proof_data.height, &tx_proof_data.block_hash) - .await?; - } else { - bail!( - "graph:{tag} at {graph_id}block_hash mismatch, from chain:{}, in contract:{}", - hex::encode(tx_proof_data.block_hash), - hex::encode(block_hash_online) - ); - } + // The SPV contract is written only by the relayer, which posts + // confirmed blocks; never overwrite it from this node's esplora view. + bail!( + "graph:{tag} at {graph_id} block_hash mismatch, from chain:{}, in contract:{}", + hex::encode(tx_proof_data.block_hash), + hex::encode(block_hash_online) + ); } Ok(tx_proof_data) } @@ -833,13 +814,6 @@ pub fn tx_reconstruct(tx: &bitcoin::Transaction) -> BitcoinTx { } // remove me later -fn is_enable_update_spv_contract() -> bool { - match std::env::var("ENABLE_UPDATE_SPV_CONTRACT") { - Ok(value) => value.to_lowercase() == "true", - Err(_) => false, - } -} - #[cfg(test)] mod tests { use crate::goat_chain::GOATClient; diff --git a/crates/client/src/graphs/graph_query.rs b/crates/client/src/graphs/graph_query.rs index 35f26d234..4d118b5a6 100644 --- a/crates/client/src/graphs/graph_query.rs +++ b/crates/client/src/graphs/graph_query.rs @@ -277,6 +277,14 @@ pub fn get_meta_data_query() -> String { builder.build() } +/// A subgraph returns numbers as strings; a malformed one is an error for the +/// caller to report, not a panic that takes the node down on every restart. +fn parse_subgraph_number(field: &str, value: &str) -> anyhow::Result { + value + .parse::() + .map_err(|_| anyhow::anyhow!("subgraph returned a non-numeric {field}: {value:?}")) +} + #[derive(Debug, Serialize, Deserialize)] pub enum UserGraphWithdrawEvent { InitWithdraw(InitWithdrawEvent), @@ -284,13 +292,13 @@ pub enum UserGraphWithdrawEvent { } impl UserGraphWithdrawEvent { - pub fn get_block_number(&self) -> i64 { + pub fn get_block_number(&self) -> anyhow::Result { match self { UserGraphWithdrawEvent::InitWithdraw(v) => { - v.block_number.parse::().expect("fail to decode block number") + parse_subgraph_number("blockNumber", &v.block_number) } UserGraphWithdrawEvent::CancelWithdraw(v) => { - v.block_number.parse::().expect("fail to decode block number") + parse_subgraph_number("blockNumber", &v.block_number) } } } @@ -378,23 +386,23 @@ pub enum WithdrawPathsEvent { } impl WithdrawPathsEvent { - pub fn get_block_number(&self) -> i64 { + pub fn get_block_number(&self) -> anyhow::Result { match self { WithdrawPathsEvent::WithdrawHappyEvent(v) => { - v.block_number.parse::().expect("fail to decode block number") + parse_subgraph_number("blockNumber", &v.block_number) } WithdrawPathsEvent::WithdrawUnhappyEvent(v) => { - v.block_number.parse::().expect("fail to decode block number") + parse_subgraph_number("blockNumber", &v.block_number) } } } - pub fn reward_amount_sats(&self) -> i64 { + pub fn reward_amount_sats(&self) -> anyhow::Result { match self { WithdrawPathsEvent::WithdrawHappyEvent(v) => { - v.reward_amount_sats.parse::().expect("fail to decode reward amount sats") + parse_subgraph_number("rewardAmountSats", &v.reward_amount_sats) } WithdrawPathsEvent::WithdrawUnhappyEvent(v) => { - v.reward_amount_sats.parse::().expect("fail to decode reward amount sats") + parse_subgraph_number("rewardAmountSats", &v.reward_amount_sats) } } } @@ -737,3 +745,23 @@ pub fn get_bridge_out_events_query( } query_builder.build() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn malformed_block_number_is_an_error_not_a_panic() { + let event = |block_number: &str| { + UserGraphWithdrawEvent::InitWithdraw(InitWithdrawEvent { + id: "id".to_string(), + transaction_hash: "0x01".to_string(), + block_number: block_number.to_string(), + instance_id: "0x02".to_string(), + graph_id: "0x03".to_string(), + }) + }; + assert_eq!(event("42").get_block_number().unwrap(), 42); + assert!(event("not-a-number").get_block_number().is_err()); + } +} diff --git a/crates/store/src/localdb/goat.rs b/crates/store/src/localdb/goat.rs index f85ae0e74..dc3fbd536 100644 --- a/crates/store/src/localdb/goat.rs +++ b/crates/store/src/localdb/goat.rs @@ -27,7 +27,7 @@ impl StorageProcessor<'_> { &mut self, watch_contract: &WatchContract, ) -> anyhow::Result<()> { - let _ = sqlx::query!( + sqlx::query!( "INSERT INTO watch_contract (contract_addr, the_graph_url, gap, from_height, status, extra, updated_at, created_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (contract_addr) DO UPDATE SET the_graph_url = excluded.the_graph_url, @@ -44,7 +44,7 @@ impl StorageProcessor<'_> { watch_contract.extra, watch_contract.updated_at, watch_contract.created_at - ).execute(self.conn()).await; + ).execute(self.conn()).await?; Ok(()) } @@ -54,14 +54,14 @@ impl StorageProcessor<'_> { status: &str, updated_at: i64, ) -> anyhow::Result<()> { - let _ = sqlx::query!( + sqlx::query!( "UPDATE watch_contract SET status = ?, updated_at = ? WHERE contract_addr = ?", status, updated_at, contract_addr, ) .execute(self.conn()) - .await; + .await?; Ok(()) } diff --git a/crates/store/src/localdb/tests.rs b/crates/store/src/localdb/tests.rs index 07950a40c..2187ba14f 100644 --- a/crates/store/src/localdb/tests.rs +++ b/crates/store/src/localdb/tests.rs @@ -731,3 +731,25 @@ async fn balance_update_does_not_overwrite_node_fields_or_a_rebound_key() { assert_eq!(nodes[0].goat_addr, "untrusted"); assert_eq!(nodes[0].reward, "42"); } + +#[tokio::test] +async fn watch_contract_writes_report_database_errors() { + let local_db = setup_db().await; + let mut storage = local_db.acquire().await.unwrap(); + let watch = crate::WatchContract { + contract_addr: "0xabc".to_string(), + the_graph_url: "http://graph".to_string(), + gap: 1, + from_height: 0, + status: "Normal".to_string(), + extra: None, + updated_at: 1, + created_at: 1, + }; + storage.upsert_watch_contract(&watch).await.unwrap(); + storage.update_watch_contract_status("0xabc", "Syncing", 2).await.unwrap(); + + sqlx::query("DROP TABLE watch_contract").execute(storage.conn()).await.unwrap(); + assert!(storage.upsert_watch_contract(&watch).await.is_err()); + assert!(storage.update_watch_contract_status("0xabc", "Normal", 3).await.is_err()); +} diff --git a/node/src/rpc_service/swap.rs b/node/src/rpc_service/swap.rs index 01642dd72..cb00c4e0e 100644 --- a/node/src/rpc_service/swap.rs +++ b/node/src/rpc_service/swap.rs @@ -112,6 +112,12 @@ fn get_swap_status_extra(status: &str) -> StatusExtra { status_extra } +/// Confirmations of a transaction mined at `tx_height`; 0 while this node's view +/// of the chain is behind the explorer that reported the height. +fn confirmations(current_height: u32, tx_height: u32) -> u32 { + current_height.saturating_add(1).saturating_sub(tx_height) +} + async fn get_claim_btc_confirm_progress( btc_client: &BTCClient, current_height: u32, @@ -121,7 +127,7 @@ async fn get_claim_btc_confirm_progress( && let Ok(tx_status) = btc_client.get_tx_status(&txid.0).await && let Some(height) = tx_status.block_height { - (current_height + 1 - height, SWAP_CLAIM_BTC_TARGET_CONFIRMATIONS) + (confirmations(current_height, height), SWAP_CLAIM_BTC_TARGET_CONFIRMATIONS) } else { (0, SWAP_CLAIM_BTC_TARGET_CONFIRMATIONS) } @@ -137,3 +143,16 @@ pub struct SwapListResponse { pub struct SwapGetResponse { pub swap: Option, } + +#[cfg(test)] +mod tests { + use super::confirmations; + + #[test] + fn confirmations_do_not_wrap_when_the_node_is_behind() { + assert_eq!(confirmations(100, 100), 1); + assert_eq!(confirmations(105, 100), 6); + assert_eq!(confirmations(98, 100), 0); + assert_eq!(confirmations(u32::MAX, 0), u32::MAX); + } +} diff --git a/node/src/scheduled_tasks/event_watch/gateway.rs b/node/src/scheduled_tasks/event_watch/gateway.rs index 4f528a027..7f2ef35f9 100644 --- a/node/src/scheduled_tasks/event_watch/gateway.rs +++ b/node/src/scheduled_tasks/event_watch/gateway.rs @@ -172,8 +172,12 @@ async fn handle_user_withdraw_events<'a>( cancel_withdraw_events.into_iter().map(UserGraphWithdrawEvent::CancelWithdraw).collect(); user_withdraw_events .extend(init_withdraw_events.into_iter().map(UserGraphWithdrawEvent::InitWithdraw)); - user_withdraw_events.sort_by_key(|v| v.get_block_number()); - for event in user_withdraw_events { + let mut user_withdraw_events = user_withdraw_events + .into_iter() + .map(|event| Ok((event.get_block_number()?, event))) + .collect::>>()?; + user_withdraw_events.sort_by_key(|(block_number, _)| *block_number); + for (_, event) in user_withdraw_events { match event { UserGraphWithdrawEvent::InitWithdraw(init_event) => { let instance_id = Uuid::from_str(&strip_hex_prefix_owned(&init_event.instance_id))?; @@ -403,7 +407,7 @@ async fn handle_withdraw_paths_events<'a>( graph_id, tx_type: tx_type.clone(), tx_hash: event.tx_hash(), - height: event.get_block_number(), + height: event.get_block_number()?, is_local: false, processing_status: GoatTxProcessingStatus::Pending.to_string(), extra: None, diff --git a/node/src/utils/evm_swap_utils.rs b/node/src/utils/evm_swap_utils.rs index e56ce68a3..414dd0d2c 100644 --- a/node/src/utils/evm_swap_utils.rs +++ b/node/src/utils/evm_swap_utils.rs @@ -254,6 +254,11 @@ fn find_claim_data( swap_contract_address: &EvmAddress, escrow_hash: &[u8; 32], ) -> anyhow::Result> { + // A reverted frame and everything under it had no effect on chain; a claim + // inside it could carry any witness. + if call.error.is_some() { + return Ok(None); + } if call.to == Some(*swap_contract_address) && let Ok(calldata) = IEscrowManagerCalls::abi_decode(&call.input) && let IEscrowManagerCalls::claim(args) = calldata @@ -353,6 +358,66 @@ fn test_hash_claim_commitment() { assert_eq!(commitment_hash, expected_hash); } +/// The claim witness of testnet3 Goat tx 0xc2b26508…e64a. +#[cfg(test)] +const TESTNET_CLAIM_WITNESS_HEX: &str = "8977831297fa2d7156898a8d26bb9e276a83cf907d739eb64ff98a0092e9250e000000023887b022\ + 17726bb36958dd595e57293fb63d508200600020bfe2760399ccb567289b120361316911b13e937aa0f2742bb7\ + 0b000000000000efaf19dc26fadb8b1cd69e76c6d1f51123f6d22407755f865b65b446cf863c3c5fbe3769f0ff\ + 0f1ad88a60f10000000000000000000000000000000000000000000017b6e253602b5f4cc25000494e876937b2\ + 636937bd246937bd2a6937bd2d6937bd306937bd6f6937bd8f6937bda66937bde76937bdf96937be07000000000\ + 00000000000000000000000000000000000000000000000000000000000008902000000018c3db50ef29dbdddf9\ + 628cd968667688cd37560ef5e64131efc3a1e4cf4e739c0100000000a1d60dfe022725000000000000225120a5d\ + 06cb76aaf6287b93a8ee73d9678e32b039354e6df4019bbd60087e347f5ccf61a010000000000225120eb3e0c2d\ + d6b344c6efaa771306a29e864c65d536dfac8f89a87680285af7acad1afc4b5d000000060000000000000000000\ + 000000000000000000000000000000000000000000003f7cfe6eed4929eea954c8e1358704c6dec19e2826f7223\ + ff7d0eff92c1addf20fd2b876e05846a100c38bd3c30619c66437abb97bc5fc21e2595fbb8a514f5341ebf1f8dd\ + 26752879de5c581667aaf183b18c1b5a111f4a24061d2c44aea2fd1"; + +#[test] +fn claims_inside_reverted_frames_are_ignored() { + use alloy::primitives::{Bytes, U256}; + use alloy::sol_types::SolCall; + + let swap = EvmAddress::repeat_byte(0x11); + let escrow = EscrowData { + offerer: EvmAddress::ZERO, + claimer: EvmAddress::ZERO, + amount: U256::from(1), + token: EvmAddress::ZERO, + flags: U256::ZERO, + claimHandler: EvmAddress::ZERO, + claimData: B256::ZERO, + refundHandler: EvmAddress::ZERO, + refundData: B256::ZERO, + securityDeposit: U256::ZERO, + claimerBounty: U256::ZERO, + depositToken: EvmAddress::ZERO, + successActionCommitment: B256::ZERO, + }; + let escrow_hash = hash_escrow_data(&escrow).0; + let input = IEscrowManager::claimCall { + escrow, + witness: Bytes::from(hex::decode(TESTNET_CLAIM_WITNESS_HEX).unwrap()), + } + .abi_encode(); + let claim_frame = |error: Option<&str>| CallFrame { + to: Some(swap), + input: input.clone().into(), + error: error.map(str::to_owned), + ..Default::default() + }; + let root = |child: CallFrame| CallFrame { + to: Some(EvmAddress::repeat_byte(0x22)), + calls: vec![child], + ..Default::default() + }; + + let reverted = root(claim_frame(Some("execution reverted"))); + assert!(find_claim_data("0x01", &reverted, &swap, &escrow_hash).unwrap().is_none()); + let succeeded = root(claim_frame(None)); + assert!(find_claim_data("0x01", &succeeded, &swap, &escrow_hash).unwrap().is_some()); +} + #[tokio::test] async fn test_find_claim_data() { unsafe { @@ -382,19 +447,7 @@ async fn test_find_claim_data() { confirmations: 2u32, btc_relay_contract: EvmAddress::from_str("0x3887B02217726bB36958Dd595e57293fB63D5082") .unwrap(), - witness: "8977831297fa2d7156898a8d26bb9e276a83cf907d739eb64ff98a0092e9250e000000023887b022\ - 17726bb36958dd595e57293fb63d508200600020bfe2760399ccb567289b120361316911b13e937aa0f2742bb7\ - 0b000000000000efaf19dc26fadb8b1cd69e76c6d1f51123f6d22407755f865b65b446cf863c3c5fbe3769f0ff\ - 0f1ad88a60f10000000000000000000000000000000000000000000017b6e253602b5f4cc25000494e876937b2\ - 636937bd246937bd2a6937bd2d6937bd306937bd6f6937bd8f6937bda66937bde76937bdf96937be07000000000\ - 00000000000000000000000000000000000000000000000000000000000008902000000018c3db50ef29dbdddf9\ - 628cd968667688cd37560ef5e64131efc3a1e4cf4e739c0100000000a1d60dfe022725000000000000225120a5d\ - 06cb76aaf6287b93a8ee73d9678e32b039354e6df4019bbd60087e347f5ccf61a010000000000225120eb3e0c2d\ - d6b344c6efaa771306a29e864c65d536dfac8f89a87680285af7acad1afc4b5d000000060000000000000000000\ - 000000000000000000000000000000000000000000003f7cfe6eed4929eea954c8e1358704c6dec19e2826f7223\ - ff7d0eff92c1addf20fd2b876e05846a100c38bd3c30619c66437abb97bc5fc21e2595fbb8a514f5341ebf1f8dd\ - 26752879de5c581667aaf183b18c1b5a111f4a24061d2c44aea2fd1" - .to_string(), + witness: TESTNET_CLAIM_WITNESS_HEX.to_string(), }; let claim_data = extract_claim_data_from_tx(&goat_client, tx_hash, &swap_contract_address, &escrow_hash)