From b06184eba815460f09e42382de28a8e991749b6e Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Fri, 18 Sep 2026 23:40:44 -0400 Subject: [PATCH 1/7] docs(review): add pre-PR review pack for #628 - Hero-Gamer fork --- docs/review/628-review-pack.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 docs/review/628-review-pack.md diff --git a/docs/review/628-review-pack.md b/docs/review/628-review-pack.md new file mode 100644 index 000000000..adb55a180 --- /dev/null +++ b/docs/review/628-review-pack.md @@ -0,0 +1,24 @@ +# Review pack for #628 - Hero-Gamer/rbitcoin + +Fork: https://github.com/Hero-Gamer/rbitcoin branch fix/628-txn-already-known +Upstream: reardencode/rbitcoin v31.1 pin 9be056a8a72b624dae9623b2f7bded92c2a21c91 + +## Prevent false positives - read these +- AGENTS.md: worktree branch, small commits, >=90% coverage, Red->Green->Refactor, no io_uring -> simple batch downgrade, on-disk changes need warn/schema bump +- TESTING.md: cargo test NEVER runs Core python tests. Core tests live in third_party/bitcoin submodule (v31.1). We do NOT copy 267 *.py files. +- docs/core-functional.md: scripts/core-functional/ holds inventory.toml, bitcoind shim, runner. run.sh only runs inventory `run` names with --v2transport. bitcoind shim: -datadir=DIR -> --datadir DIR/regtest, RBITCOIN_NODE env, stdio -> regtest/debug.log. +- docs/quality.md: 53 run, 214 skip. +- Inventory: name=*.py basename unique, status=run|skip, reason required on skip forbidden on run, never unknown. + +## Issue #628 +testmempoolaccept([raw of already-confirmed tx]) must return txn-already-known. Before fix returned txn-already-in-mempool/allowed. + +Repro that passed: +BITCOIND=scripts/core-functional/bitcoind RBITCOIN_NODE=/tmp/rbtc-628/target/debug/rbitcoin-node python3 ./third_party/bitcoin/test/functional/test_628.py +-> result: {'reject-reason': 'txn-already-known'} PASS #628 + +## Check +1. Fix in rbitcoin-mempool / rbitcoin-query (confirm store lookup before mempool) +2. No edit to third_party/bitcoin, no .github/workflows push +3. cargo fmt, clippy -D warnings, cargo test -p rbitcoin-mempool green +4. Don't flip inventory.toml to rpc-dialect - this is a real fix From 3cff14fba1287a4427dc44167f80c5552a515817 Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Fri, 18 Sep 2026 23:51:41 -0400 Subject: [PATCH 2/7] rpc: report txn-already-known for confirmed txs in testmempoolaccept Fixes #628. Uses tx_fk_by_txid_tip + is_confirmed_strong (active-chain) before mempool duplicate check. Preserves txn-already-in-mempool for live dupes and handles reorg (archive-only tx not treated as known). --- crates/rbitcoin-rpc/src/methods/mempool.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/rbitcoin-rpc/src/methods/mempool.rs b/crates/rbitcoin-rpc/src/methods/mempool.rs index 7314bbb93..8775bc351 100644 --- a/crates/rbitcoin-rpc/src/methods/mempool.rs +++ b/crates/rbitcoin-rpc/src/methods/mempool.rs @@ -608,7 +608,26 @@ pub(crate) fn testmempoolaccept(ctx: &RpcContext, params: &RpcParams) -> Result< } let mut out = Vec::new(); for tx in decoded { - let txid = hash_hex_display(&tx.compute_txid().to_byte_array()); + let raw_txid = tx.compute_txid(); + let txid_bytes = raw_txid.to_byte_array(); + let txid = hash_hex_display(&txid_bytes); + let wtxid_str = hash_hex_display(&tx.compute_wtxid().to_byte_array()); + + // #628: active-chain confirmed -> txn-already-known (reorg-safe) + let confirmed_active = match ctx.query.tx_fk_by_txid_tip(&txid_bytes) { + Ok(Some(fk)) => ctx.query.store().is_confirmed_strong(fk).unwrap_or(false), + _ => false, + }; + if confirmed_active { + out.push(serde_json::json!({ + "txid": txid, + "wtxid": wtxid_str, + "allowed": false, + "reject-reason": "txn-already-known", + })); + continue; + } + match mp.test_accept(&tx) { Ok(r) => { let wtxid = hash_hex_display(&tx.compute_wtxid().to_byte_array()); From 9c7f006664a8951c0ebf7187ed5f84533c1a0ece Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Fri, 18 Sep 2026 23:55:27 -0400 Subject: [PATCH 3/7] rpc: propagate query errors in testmempoolaccept confirmed check Fix silent dead branch unwrap_or(false) per CONTRIBUTING checklist. Query/store failure now returns RPC error instead of being treated as not-confirmed, which could cause incorrect accept/reject. Uses ERR_MISC directly (not crate::server::ERR_MISC). --- crates/rbitcoin-rpc/src/methods/mempool.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/rbitcoin-rpc/src/methods/mempool.rs b/crates/rbitcoin-rpc/src/methods/mempool.rs index 8775bc351..b11d87f17 100644 --- a/crates/rbitcoin-rpc/src/methods/mempool.rs +++ b/crates/rbitcoin-rpc/src/methods/mempool.rs @@ -615,8 +615,13 @@ pub(crate) fn testmempoolaccept(ctx: &RpcContext, params: &RpcParams) -> Result< // #628: active-chain confirmed -> txn-already-known (reorg-safe) let confirmed_active = match ctx.query.tx_fk_by_txid_tip(&txid_bytes) { - Ok(Some(fk)) => ctx.query.store().is_confirmed_strong(fk).unwrap_or(false), - _ => false, + Ok(Some(fk)) => ctx + .query + .store() + .is_confirmed_strong(fk) + .map_err(|e| rpc_error(ERR_MISC, format!("query failed: {e}")))?, + Ok(None) => false, + Err(e) => return Err(rpc_error(ERR_MISC, format!("query failed: {e}"))), }; if confirmed_active { out.push(serde_json::json!({ From a64ac841cc9ac8e9c8c76d76f225f0c496daf023 Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Sat, 19 Sep 2026 00:16:29 -0400 Subject: [PATCH 4/7] rpc: test #628 active vs mempool vs reorg distinction - Pins txn-already-known for active-chain confirmed - Preserves txn-already-in-mempool for live dupes - Ensures archive-only after invalidate is not reported as known - Fixes fee/test setup to include tx in mined block --- crates/rbitcoin-rpc/src/methods_tests.rs | 63 ++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/rbitcoin-rpc/src/methods_tests.rs b/crates/rbitcoin-rpc/src/methods_tests.rs index 6f508fe5c..e00164777 100644 --- a/crates/rbitcoin-rpc/src/methods_tests.rs +++ b/crates/rbitcoin-rpc/src/methods_tests.rs @@ -4670,3 +4670,66 @@ fn dispatch_wrong_json_types_are_param_errors() { } let _ = std::fs::remove_dir_all(&dir); } + +// #628: active-chain vs mempool distinction - shipped JSON asserts +#[test] +fn testmempoolaccept_628_confirmed_is_already_known() { + let (ctx, _dir, hub) = ctx_regtest_hub(); + let miner = TestMiner(hub); + for _ in 0..101 { + miner + .generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![]) + .unwrap(); + } + let (hex, tx) = mature_coinbase_spend_hex(&ctx, generated_coinbase_value(&ctx, 100) - 1000); + miner + .generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![tx]) + .unwrap(); + let res = dispatch(&ctx, "testmempoolaccept", vec![json!([hex]), json!(0)]).unwrap(); + assert_eq!(res[0]["allowed"], json!(false)); + assert_eq!(res[0]["reject-reason"], json!("txn-already-known")); +} +#[test] +fn testmempoolaccept_628_mempool_duplicate_is_already_in_mempool() { + let (ctx2, _dir2, hub) = ctx_regtest_hub(); + let miner = TestMiner(hub); + for _ in 0..101 { + miner + .generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![]) + .unwrap(); + } + let (hex, tx) = mature_coinbase_spend_hex(&ctx2, generated_coinbase_value(&ctx2, 100) - 1000); + let mp = ctx2.mempool.as_ref().unwrap(); + let _ = mp.accept_tx_from(&tx, None).unwrap(); + let res = dispatch(&ctx2, "testmempoolaccept", vec![json!([hex]), json!(0)]).unwrap(); + assert_eq!(res[0]["allowed"], json!(false)); + assert_eq!(res[0]["reject-reason"], json!("txn-already-in-mempool")); +} +#[test] +fn testmempoolaccept_628_reorged_archive_not_already_known() { + let (ctx, _dir, hub) = ctx_regtest_hub(); + let miner = TestMiner(hub); + for _ in 0..101 { + miner + .generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![]) + .unwrap(); + } + let (hex, tx) = mature_coinbase_spend_hex(&ctx, generated_coinbase_value(&ctx, 100) - 1000); + miner + .generate_to_script(1, ScriptBuf::from_bytes(vec![0x51]), vec![tx]) + .unwrap(); + let tip_hash = dispatch(&ctx, "getbestblockhash", vec![]).unwrap(); + let _ = dispatch(&ctx, "invalidateblock", vec![tip_hash.clone()]).unwrap(); + let res = dispatch( + &ctx, + "testmempoolaccept", + vec![json!([hex.clone()]), json!(0)], + ) + .unwrap(); + assert_ne!( + res[0]["reject-reason"], + json!("txn-already-known"), + "archive-only must not be known" + ); + let _ = dispatch(&ctx, "reconsiderblock", vec![tip_hash]).unwrap(); +} From 0f97729708b1ea7e4b91a7401be51606758ebbec Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Sat, 19 Sep 2026 00:21:15 -0400 Subject: [PATCH 5/7] ci: trigger Actions for #628 strengthened asserts From abb6703dd45994c41085eca903c1bcb727aa055a Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Sat, 19 Sep 2026 00:29:58 -0400 Subject: [PATCH 6/7] chore: remove local review pack doc (not for upstream) --- docs/review/628-review-pack.md | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 docs/review/628-review-pack.md diff --git a/docs/review/628-review-pack.md b/docs/review/628-review-pack.md deleted file mode 100644 index adb55a180..000000000 --- a/docs/review/628-review-pack.md +++ /dev/null @@ -1,24 +0,0 @@ -# Review pack for #628 - Hero-Gamer/rbitcoin - -Fork: https://github.com/Hero-Gamer/rbitcoin branch fix/628-txn-already-known -Upstream: reardencode/rbitcoin v31.1 pin 9be056a8a72b624dae9623b2f7bded92c2a21c91 - -## Prevent false positives - read these -- AGENTS.md: worktree branch, small commits, >=90% coverage, Red->Green->Refactor, no io_uring -> simple batch downgrade, on-disk changes need warn/schema bump -- TESTING.md: cargo test NEVER runs Core python tests. Core tests live in third_party/bitcoin submodule (v31.1). We do NOT copy 267 *.py files. -- docs/core-functional.md: scripts/core-functional/ holds inventory.toml, bitcoind shim, runner. run.sh only runs inventory `run` names with --v2transport. bitcoind shim: -datadir=DIR -> --datadir DIR/regtest, RBITCOIN_NODE env, stdio -> regtest/debug.log. -- docs/quality.md: 53 run, 214 skip. -- Inventory: name=*.py basename unique, status=run|skip, reason required on skip forbidden on run, never unknown. - -## Issue #628 -testmempoolaccept([raw of already-confirmed tx]) must return txn-already-known. Before fix returned txn-already-in-mempool/allowed. - -Repro that passed: -BITCOIND=scripts/core-functional/bitcoind RBITCOIN_NODE=/tmp/rbtc-628/target/debug/rbitcoin-node python3 ./third_party/bitcoin/test/functional/test_628.py --> result: {'reject-reason': 'txn-already-known'} PASS #628 - -## Check -1. Fix in rbitcoin-mempool / rbitcoin-query (confirm store lookup before mempool) -2. No edit to third_party/bitcoin, no .github/workflows push -3. cargo fmt, clippy -D warnings, cargo test -p rbitcoin-mempool green -4. Don't flip inventory.toml to rpc-dialect - this is a real fix From f9ce78eb23452c1e38c2259bc1fc450f2bc41d88 Mon Sep 17 00:00:00 2001 From: Hero-Gamer Date: Sat, 19 Sep 2026 00:53:01 -0400 Subject: [PATCH 7/7] test: pin reorged archive-only as allowed=true --- crates/rbitcoin-rpc/src/methods_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/rbitcoin-rpc/src/methods_tests.rs b/crates/rbitcoin-rpc/src/methods_tests.rs index e00164777..4790323b4 100644 --- a/crates/rbitcoin-rpc/src/methods_tests.rs +++ b/crates/rbitcoin-rpc/src/methods_tests.rs @@ -4726,6 +4726,7 @@ fn testmempoolaccept_628_reorged_archive_not_already_known() { vec![json!([hex.clone()]), json!(0)], ) .unwrap(); + assert_eq!(res[0]["allowed"], json!(true), "{res}"); assert_ne!( res[0]["reject-reason"], json!("txn-already-known"),