diff --git a/src/chainstate/stacks/db/blocks.rs b/src/chainstate/stacks/db/blocks.rs index aec213a220..aa5d7ab28a 100644 --- a/src/chainstate/stacks/db/blocks.rs +++ b/src/chainstate/stacks/db/blocks.rs @@ -142,7 +142,12 @@ pub enum MemPoolRejection { NoCoinbaseViaMempool, NoSuchChainTip(ConsensusHash, BlockHeaderHash), ConflictingNonceInMempool, - TooMuchChaining, + TooMuchChaining { + max_nonce: u64, + actual_nonce: u64, + principal: PrincipalData, + is_origin: bool, + }, DBError(db_error), Other(String), } @@ -155,9 +160,22 @@ impl MemPoolRejection { DeserializationFailure(e) => { ("Deserialization", Some(json!({"message": e.to_string()}))) } - TooMuchChaining => ( + TooMuchChaining { + max_nonce, + actual_nonce, + principal, + is_origin, + .. + } => ( "TooMuchChaining", - Some(json!({"message": "Nonce would exceed chaining limit in mempool"})), + Some( + json!({"message": "Nonce would exceed chaining limit in mempool", + "expected": max_nonce, + "actual": actual_nonce, + "principal": principal.to_string(), + "is_origin": is_origin + }), + ), ), FailedToValidate(e) => ( "SignatureValidation", @@ -4879,7 +4897,6 @@ impl StacksChainState { /// unconfirmed microblock stream trailing off of it. pub fn will_admit_mempool_tx( &mut self, - mempool_conn: &DBConn, current_consensus_hash: &ConsensusHash, current_block: &BlockHeaderHash, tx: &StacksTransaction, @@ -4922,14 +4939,7 @@ impl StacksChainState { let current_tip = StacksChainState::get_parent_index_block(current_consensus_hash, current_block); let res = match self.with_read_only_clarity_tx(&NULL_BURN_STATE_DB, ¤t_tip, |conn| { - StacksChainState::can_include_tx( - mempool_conn, - conn, - &conf, - has_microblock_pubk, - tx, - tx_size, - ) + StacksChainState::can_include_tx(conn, &conf, has_microblock_pubk, tx, tx_size) }) { Some(r) => r, None => Err(MemPoolRejection::NoSuchChainTip( @@ -4950,7 +4960,6 @@ impl StacksChainState { &tx.txid(), mismatch_error.expected, mismatch_error.actual); self.with_read_only_unconfirmed_clarity_tx(&NULL_BURN_STATE_DB, |conn| { StacksChainState::can_include_tx( - mempool_conn, conn, &conf, has_microblock_pubk, @@ -4969,8 +4978,7 @@ impl StacksChainState { /// Given an outstanding clarity connection, can we append the tx to the chain state? /// Used when mining transactions. - pub fn can_include_tx( - mempool: &DBConn, + fn can_include_tx( clarity_connection: &mut T, chainstate_config: &DBConfig, has_microblock_pubkey: bool, @@ -4997,46 +5005,35 @@ impl StacksChainState { let (origin, payer) = match StacksChainState::check_transaction_nonces(clarity_connection, &tx, true) { Ok(x) => x, - Err((mut e, (origin, payer))) => { - // let's see if the tx has matching nonce in the mempool - // -> you can _only_ replace-by-fee or replace-across-fork - // for nonces that increment the current chainstate's nonce. - // if there are "chained" transactions in the mempool, - // this check won't admit a rbf/raf for those. - let origin_addr = tx.origin_address(); - let origin_nonce = tx.get_origin().nonce(); - let origin_next_nonce = - MemPoolDB::get_next_nonce_for_address(mempool, &origin_addr)?; - if origin_next_nonce < origin.nonce { + // if errored, check if MEMPOOL_TX_CHAINING would admit this TX + Err((e, (origin, payer))) => { + // if the nonce is less than expected, then TX_CHAINING would not allow in any case + if e.actual < e.expected { return Err(e.into()); } - if origin_next_nonce - origin.nonce >= MAXIMUM_MEMPOOL_TX_CHAINING { - return Err(MemPoolRejection::TooMuchChaining); - } - if origin_nonce != origin_next_nonce { - e.is_origin = true; - e.principal = origin_addr.into(); - e.expected = origin_next_nonce; - e.actual = origin_nonce; - return Err(e.into()); + + let tx_origin_nonce = tx.get_origin().nonce(); + + let origin_max_nonce = origin.nonce + 1 + MAXIMUM_MEMPOOL_TX_CHAINING; + if origin_max_nonce < tx_origin_nonce { + return Err(MemPoolRejection::TooMuchChaining { + max_nonce: origin_max_nonce, + actual_nonce: tx_origin_nonce, + principal: tx.origin_address().into(), + is_origin: true, + }); } if let Some(sponsor_addr) = tx.sponsor_address() { - let sponsor_nonce = tx.get_payer().nonce(); - let sponsor_next_nonce = - MemPoolDB::get_next_nonce_for_address(mempool, &sponsor_addr)?; - if sponsor_next_nonce < payer.nonce { - return Err(e.into()); - } - if sponsor_next_nonce - payer.nonce >= MAXIMUM_MEMPOOL_TX_CHAINING { - return Err(MemPoolRejection::TooMuchChaining); - } - if sponsor_nonce != sponsor_next_nonce { - e.is_origin = false; - e.principal = sponsor_addr.into(); - e.expected = sponsor_next_nonce; - e.actual = sponsor_nonce; - return Err(e.into()); + let tx_sponsor_nonce = tx.get_payer().nonce(); + let sponsor_max_nonce = payer.nonce + 1 + MAXIMUM_MEMPOOL_TX_CHAINING; + if sponsor_max_nonce < tx_sponsor_nonce { + return Err(MemPoolRejection::TooMuchChaining { + max_nonce: sponsor_max_nonce, + actual_nonce: tx_sponsor_nonce, + principal: sponsor_addr.into(), + is_origin: false, + }); } } (origin, payer) diff --git a/src/core/mempool.rs b/src/core/mempool.rs index 88ff003c12..4569d48321 100644 --- a/src/core/mempool.rs +++ b/src/core/mempool.rs @@ -79,18 +79,11 @@ impl MemPoolAdmitter { pub fn will_admit_tx( &mut self, - mempool_conn: &DBConn, chainstate: &mut StacksChainState, tx: &StacksTransaction, tx_size: u64, ) -> Result<(), MemPoolRejection> { - chainstate.will_admit_mempool_tx( - mempool_conn, - &self.cur_consensus_hash, - &self.cur_block, - tx, - tx_size, - ) + chainstate.will_admit_mempool_tx(&self.cur_consensus_hash, &self.cur_block, tx, tx_size) } } @@ -804,8 +797,15 @@ impl MemPoolDB { true } else { // there's a >= fee tx in this fork, cannot add - info!("TX conflicts with sponsor/origin nonce in same fork with >= fee: new_txid={}, old_txid={}, origin_addr={}, origin_nonce={}, sponsor_addr={}, sponsor_nonce={}, new_fee={}, old_fee={}", - txid, prior_tx.txid, origin_address, origin_nonce, sponsor_address, sponsor_nonce, estimated_fee, prior_tx.estimated_fee); + info!("TX conflicts with sponsor/origin nonce in same fork with >= fee"; + "new_txid" => %txid, + "old_txid" => %prior_tx.txid, + "origin_addr" => %origin_address, + "origin_nonce" => origin_nonce, + "sponsor_addr" => %sponsor_address, + "sponsor_nonce" => sponsor_nonce, + "new_fee" => estimated_fee, + "old_fee" => prior_tx.estimated_fee); false } } else { @@ -960,9 +960,7 @@ impl MemPoolDB { mempool_tx .admitter .set_block(&block_hash, (*consensus_hash).clone()); - mempool_tx - .admitter - .will_admit_tx(&mempool_tx.tx, chainstate, &tx, len)?; + mempool_tx.admitter.will_admit_tx(chainstate, &tx, len)?; } MemPoolDB::try_add_tx( diff --git a/testnet/stacks-node/src/tests/integrations.rs b/testnet/stacks-node/src/tests/integrations.rs index bdb8532d54..d74bc827a8 100644 --- a/testnet/stacks-node/src/tests/integrations.rs +++ b/testnet/stacks-node/src/tests/integrations.rs @@ -625,7 +625,7 @@ fn integration_test_get_info() { eprintln!("Test: POST {} (invalid)", path); // tx_xfer_invalid is 180 bytes long - let tx_xfer_invalid = make_stacks_transfer(&spender_sk, (round + 10).into(), 200, // bad nonce + let tx_xfer_invalid = make_stacks_transfer(&spender_sk, (round + 30).into(), 200, // bad nonce &StacksAddress::from_string(ADDR_4).unwrap().into(), 456); let tx_xfer_invalid_tx = StacksTransaction::consensus_deserialize(&mut &tx_xfer_invalid[..]).unwrap(); @@ -757,12 +757,12 @@ fn contract_stx_transfer() { ) .unwrap(); } - // this one should fail: + // this one should fail because the nonce is already in the mempool let xfer_to_contract = - make_stacks_transfer(&sk_3, 3, 200, &contract_identifier.clone().into(), 1000); + make_stacks_transfer(&sk_3, 3, 190, &contract_identifier.clone().into(), 1000); let xfer_to_contract = StacksTransaction::consensus_deserialize(&mut &xfer_to_contract[..]).unwrap(); - let result = match tenure + match tenure .mem_pool .submit( &mut chainstate_copy, @@ -772,10 +772,11 @@ fn contract_stx_transfer() { ) .unwrap_err() { - MemPoolRejection::TooMuchChaining => true, - _ => false, + MemPoolRejection::ConflictingNonceInMempool => (), + e => { + panic!("{:?}", e) + } }; - assert!(result); } return; @@ -1399,8 +1400,8 @@ fn mempool_errors() { eprintln!("Test: POST {} (invalid)", path); let tx_xfer_invalid = make_stacks_transfer( &spender_sk, - 3, - 200, // bad nonce + 30, // bad nonce -- too much chaining + 200, &send_to, 456, ); @@ -1425,15 +1426,18 @@ fn mempool_errors() { res.get("error").unwrap().as_str().unwrap(), "transaction rejected" ); - assert_eq!(res.get("reason").unwrap().as_str().unwrap(), "BadNonce"); + assert_eq!( + res.get("reason").unwrap().as_str().unwrap(), + "TooMuchChaining" + ); let data = res.get("reason_data").unwrap(); assert_eq!(data.get("is_origin").unwrap().as_bool().unwrap(), true); assert_eq!( data.get("principal").unwrap().as_str().unwrap(), &spender_addr.to_string() ); - assert_eq!(data.get("expected").unwrap().as_i64().unwrap(), 0); - assert_eq!(data.get("actual").unwrap().as_i64().unwrap(), 3); + assert_eq!(data.get("expected").unwrap().as_i64().unwrap(), 26); + assert_eq!(data.get("actual").unwrap().as_i64().unwrap(), 30); let tx_xfer_invalid = make_stacks_transfer( &spender_sk, diff --git a/testnet/stacks-node/src/tests/mempool.rs b/testnet/stacks-node/src/tests/mempool.rs index 8725edf11d..13ac31956a 100644 --- a/testnet/stacks-node/src/tests/mempool.rs +++ b/testnet/stacks-node/src/tests/mempool.rs @@ -196,8 +196,7 @@ fn mempool_setup_chainstate() { let chainstate_path = { CHAINSTATE_PATH.lock().unwrap().clone().unwrap() }; - let mempool = MemPoolDB::open(false, TESTNET_CHAIN_ID, &chainstate_path).unwrap(); - let mempool_conn = mempool.conn(); + let _mempool = MemPoolDB::open(false, TESTNET_CHAIN_ID, &chainstate_path).unwrap(); if round == 3 { let block_header = chain_tip.metadata.clone(); @@ -213,13 +212,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap(); let tx_bytes = make_contract_call( @@ -234,26 +227,14 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap(); let tx_bytes = make_stacks_transfer(&contract_sk, 5, 200, &other_addr, 1000); let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap(); // bad signature @@ -261,13 +242,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!( @@ -303,13 +278,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); assert!(if let MemPoolRejection::BadAddressVersionByte = e { @@ -332,13 +301,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); assert!(if let MemPoolRejection::BadAddressVersionByte = e { true @@ -351,13 +314,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::FeeTooLow(0, _) = e { @@ -371,13 +328,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::BadNonces(_) = e { @@ -391,13 +342,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::NotEnoughFunds(111000, 99500) = e { @@ -410,13 +355,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::NotEnoughFunds(100700, 99500) = e { @@ -437,13 +376,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::NoSuchContract = e { @@ -464,13 +397,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::NoSuchPublicFunction = e { @@ -491,13 +418,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::BadFunctionArgument(_) = e { @@ -511,13 +432,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::ContractAlreadyExists(_) = e { @@ -546,13 +461,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!( @@ -583,13 +492,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::InvalidMicroblocks = e { @@ -621,13 +524,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!( @@ -642,13 +539,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); eprintln!("Err: {:?}", e); assert!(if let MemPoolRejection::NoCoinbaseViaMempool = e { @@ -700,13 +591,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap(); let contract_id = QualifiedContractIdentifier::new( @@ -727,13 +612,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap(); let contract_id = QualifiedContractIdentifier::new( @@ -754,13 +633,7 @@ fn mempool_setup_chainstate() { let tx = StacksTransaction::consensus_deserialize(&mut tx_bytes.as_slice()).unwrap(); let e = chain_state - .will_admit_mempool_tx( - mempool_conn, - consensus_hash, - block_hash, - &tx, - tx_bytes.len() as u64, - ) + .will_admit_mempool_tx(consensus_hash, block_hash, &tx, tx_bytes.len() as u64) .unwrap_err(); assert!(if let MemPoolRejection::BadFunctionArgument(_) = e { true