Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.Sign up
Fix and improve txn_doublespend.py test #5881
The first commit illustrates a problem with the current txn_doublespend.py test. The test relies on unsupportable behavior in the wallet's SyncMetaData method. The test is made to produce inconsistent results by making a small change to specific test "amount" parameters.
The second commit introduces a primitive method CTransaction::IsEquivalentTo and uses it in the wallet to ensure that SyncMetaData is only called for malleability clones, not general conflict (doublespend) transactions. This makes the output of the illustrative version of txn_doublespend.py consistent and predictable.
The third commit is a revised txn_doublespend.py test that does not rely on broken SyncMetaData. Instead, it expects the fixed version of SyncMetaData. It also removes a dependency on the soon-to-be-deprecated accounting "move" method.
I don't like the new unit test-- you are testing the new "" account gets debited on a non-malleated doublespend.
But our wallet code will never produce a non-malleated double-spend (at least not without the user jumping through some hoops to copy the wallet to two machines, etc etc etc).
I'd be happier if the new unit test tested the new code, and created a malleated version of the spend-to-foo and made sure The Right Thing happened.
I'm also uncertain about changing the SyncMetaData behavior to only work for malleated transactions, but I think I can live with that (mostly because what you get depends on the order your node sees transactions, accounts are being deprecated, and non-malleated double-spends of a "spendfrom" should never happen unless you jump through hoops).
Per @gavinandresen review:
The new test relies on setting nlocktime via createrawtransaction. There is also a stand-alone #5936 for this change.
Define CTransaction::IsEquivalentTo(const CTransaction& tx) True if only scriptSigs are different. In other words, true if the two transactions are malleability clones. In other words, true if the two transactions have the same effect on the outside universe. In the wallet, only SyncMetaData for equivalent transactions.
Remove reliance on accounting "move" ledger entries. Instead, create funding transactions (and deal with fee complexities). Do not rely on broken SyncMetaData. Instead expect double-spend amount to be debited from the default "" account.
Bitcoin 0.12 test PRs 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6337 - bitcoin/bitcoin#5881 - bitcoin/bitcoin#6365 - bitcoin/bitcoin#6390 - bitcoin/bitcoin#6414 - bitcoin/bitcoin#5515 - bitcoin/bitcoin#6287 (partial, remainder included in bitcoin/bitcoin#6703) - bitcoin/bitcoin#6465 Part of #2074.
Bitcoin 0.12 test PRs 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6337 - bitcoin/bitcoin#5881 - bitcoin/bitcoin@3f16971 - bitcoin/bitcoin#6365 - bitcoin/bitcoin@56dc704 - bitcoin/bitcoin#6390 - bitcoin/bitcoin#6414 - bitcoin/bitcoin#5515 - bitcoin/bitcoin#6287 (partial, remainder included in bitcoin/bitcoin#6703) - bitcoin/bitcoin#6465 Part of #2074.