Repository navigation
consensus: handle a txid repeated inside one confirm batch like Core - #1061
Conversation
Below BIP34 a coinbase can repeat once its first copy is spent. Core 31.1 accepts a chain where h1 creates X, h102 spends X:0, h103 creates X again and h203 spends the new X:0. rbitcoin could not connect it whenever those blocks shared a confirm batch, and IBD halted. - The load planner resolves a spend to the newest copy created before it, then to an outside copy, and only then to a later in-batch copy, which structural still rejects as not yet a coin. - Block assembly and structural validation free the per-batch spent marks of a txid when it is created again, so its new outputs can be spent. - BIP30 counts spends made earlier in the same batch when it checks a connected earlier copy. The repeat bookkeeping only runs below BIP34. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3ab9684 to
cc22910
Compare
There was a problem hiding this comment.
Summary
The top commit teaches one confirm batch to spend the newest copy of a txid created before that spend, to clear that txid's spent marks when the txid is created again, and to count same-batch spends in BIP30. Below BIP34 that matches Core, including a third recreate in the same batch: the store's first fk is still treated as spent because the newer copy's spend re-fills pending_spent after the previous release. Above BIP34 the new release is skipped. A BIP34-compliant coinbase can still repeat a pre-BIP34 coinbase (Core's block 1,983,702 case, and block 490,897 where BIP30 is off). If the old outputs were spent earlier in the same pending set, Core accepts a later spend of the new coins and this code rejects it.
Issue counts by severity
- bugs: 2
- suggestions: 1
- nits: 0
A pre-BIP34 coinbase that pushed a later height can repeat after BIP34. The assemble release now runs for every create, and the write stage keys its spent set by outpoint and the copy spent, so a spend of the new copy is not taken for a double spend of the old one. Adds a BIP34-era batch-split journey and a merged-write journey that drives the IBD append_contiguous path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…IP34 cc22910 stopped recording creates for the in-batch BIP30 check above BIP34. A spend tx made after BIP34 can still repeat byte for byte once a pre-BIP34 coinbase that pushed a later height is mined again. Core rejects the repeat while the first copy's output is unspent; in one batch we accepted it. Master recorded every create; this restores that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Summary
The previous review items are fixed on 17cf96a1: assemble releases repeated outpoints on every create (no BIP34 gate), a second spend of a different copy is not PrevoutSpent when both create fks are real, the no-recreate test accepts only PrevoutSpent / double spend, and batch_creates records every create again so an above-BIP34 repeat of a still-unspent spend tx is bad-txns-BIP30. batch_copy_before still binds a spend to the newest fk created before it, and production structural_run always passes real tx_fks. The new equality rule does not match Core once a same-block spend or a third copy is in the write batch. Same-block spends still store Fk::NULL, so a later spend of that coin in a merged write is not a double spend, and the store BIP30 loop treats the connected copy as unspent after a later copy's spend overwrites the mark.
Issue counts by severity
- bugs: 2
- suggestions: 0
- nits: 0
|
The archived-retry bind from #1063 is commit
Nothing else in #1063 needs a follow-up. Its BIP34 |
ca552a8 keyed write-batch spends by create fk. A same-block spend has no create fk (NULL), so a later spend of that coin in a merged write was not a double spend, and BIP30 saw a copy spent in its own block as unspent. The connected-copy BIP30 check also missed a spend that a later copy's spend replaced. A same-block spend now records its spender's fk. Two spends hit the same copy when their fks match or have the same height in the batch (one block holds one copy of a txid). In-batch BIP30 compares that height with the earlier copy's; the connected-copy check counts any spend in the batch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@rearden-grok please re-review the latest commit (4d32879). |
A structural failure archives the bodies and leaves the tip unmoved. The retry has no fresh plan, so the stamp used to keep one fk per txid and bind the later copy. It now uses the same order as the planner: the newest copy created before the spend, then a connected copy, then a later in-batch copy.
|
@rearden-grok Done. I cherry-picked af29ab3 onto this branch as e98f388, with you kept as the author. Before picking it, I checked the bug on 4d32879: your test fails there, with |
There was a problem hiding this comment.
Summary
Both bugs from review 5561226268 are fixed on 4d328799. A same-block spend now stores the spender's fk, and same_copy treats two marks as one copy when the fks are equal or RunCreateHeight gives them the same height. That matches Core: one block holds one copy of a txid, a later spend of that coin in the same write is a double spend, and a spend of a newer copy is not. The connected-copy BIP30 check uses pending_spent.contains_key, so creating copy C still succeeds after copy B's spend overwrites A's mark, while an unspent output of the newest copy still fails the in-batch height check. structural_run builds RunCreateHeight from the same tx_fks assemble stamps as spend_fk, so get(spend_fk) is not None for a real spend. connect_merged puts the same-block spend and the later respend in different load waves and one structural_run, and connect_in_batches cuts &[101] and &[101, 204] commit A before the write that spends A, creates B, spends B, and creates C.
e98f3885 is the archived retry on that tip. The fresh plan and stamp_parent_pin_archived / spend_edges_from_stamp share batch_copy_before: newest copy created before the spend, then a connected copy, then a later in-batch copy. Same-batch idents keep every copy, not only the latest. repeated_coinbase_archived_retry_connects_the_valid_prefix fails the immature h150 spend, leaves the tip at genesis, and connects h1..=h103 through the archived stamp.
Issue counts by severity
- bugs: 0
- suggestions: 0
- nits: 0
Summary
Follow-up to #1059, same batch-vs-one-block class.
A coinbase can repeat once its first copy is spent: below BIP34, or after BIP34 when an old coinbase already pushed the later height (Core's 490,897 / 1,983,702 notes). Bitcoin Core 31.1 accepts such chains. rbitcoin handles them one block at a time, but not when the blocks share a confirm batch, and IBD halts. Three places assumed a txid shows up once per batch:
Live networks are not known to produce this, so the risk is small, but it's another batch-vs-one-block gap.
Evidence
Same chains in Core 31.1 and rbitcoin. rbitcoin runs them split into confirm batches many ways, and through IBD's merged write (
append_contiguous):bad-txns-BIP30bad-txns-inputs-missingorspentbad-txns-premature-spend-of-coinbasebad-txns-BIP30bad-txns-inputs-missingorspentbad-txns-BIP30MissingPrevout,double spend in block,PrevoutSpent, orbad-txns-BIP30). A fresh master node syncing the first chain over P2P from Core 31.1 halts at height 54 (another run: 35).After: 6 of 6 syncs from the same Core node reach 203 with Core's tip hash.
MissingPrevout. IBD's isolate retry takes this path.repeated_coinbase_archived_retry_connects_the_valid_prefix,repeated_coinbase_chain_*,bip34_era_*,repeated_coinbase_connects_when_ibd_merges_load_batches,respend_of_a_same_block_spend_*,repeated_tx_spent_in_its_own_block_*,third_copy_*. Removing any one fix makes one of them fail.rbitcoin-querylib tests, and the consensus journeys pass locally.Merge Danger
Door: two-way
Blast Radius: consensus. Block confirm when a txid repeats inside one batch. Batches with no repeated txid behave as before. The assembly now does one extra set lookup per output.
🤖 Generated with Claude Code