Move recently introduced CTransAction::IsEquivalentTo to CWalletTx #6365

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2015_06_move_equivalent_to_wallettx changing 4 files +12 −12

laanwj commented at 7:01 PM on July 2, 2015: member

CTransAction::IsEquivalentTo was introduced in #5881. This functionality is only used in the wallet, and should never have been added to the primitive transaction type.

Move recently introduced CTransAction::IsEquivalentTo to CWalletTx

CTransAction::IsEquivalentTo was introduced in #5881.
This functionality is only useful to the wallet, and should never have
been added to the primitive transaction type.

5a7304b69d

laanwj added the label Refactoring on Jul 2, 2015
jonasschnelli commented at 7:13 PM on July 2, 2015: contributor

utACK.
petertodd commented at 12:53 AM on July 3, 2015: contributor

utACK
dgenr8 commented at 4:47 AM on July 3, 2015: contributor

There is nothing wallet-specific about the method, and in fact it is needed in P2P code in #4570 (though the IsEquivalentTo implementation there is in need of update).
laanwj commented at 5:37 AM on July 3, 2015: member
@dgenr8 I was more concerned with this belonging in the consensus library than anything else. We've been trying to detach methods from the base transaction data structures esp. those not used in consensus for a while. So this was a clear step in the wrong way.

If you do want to use it outside the wallet there are other solutions. Most straightforward is to make it an utility function that is not bound to any class:
```
bool AreTransactionsEquivalent(const CTransaction& txa, const CTransaction& txb) const
{
   CMutableTransaction tx1 = txa;
   CMutableTransaction tx2 = txb;
   for (unsigned int i = 0; i < tx1.vin.size(); i++) tx1.vin[i].scriptSig = CScript();
   for (unsigned int i = 0; i < tx2.vin.size(); i++) tx2.vin[i].scriptSig = CScript();
   return CTransaction(tx1) == CTransaction(tx2);
}
```
... but I wouldn't know where exactly to put it, then. Probably with other utility functions used for policy and wallet only.

Unless you have a good suggestion where to put it now, I'd say we should move ahead with this and then you can solve the issue in #4570 (if we are still going to merge that).
dgenr8 commented at 6:15 AM on July 3, 2015: contributor

Ok, no problem.
laanwj merged this on Jul 6, 2015
laanwj closed this on Jul 6, 2015
laanwj referenced this in commit 086cb336cd on Jul 6, 2015
dgenr8 commented at 5:20 PM on July 12, 2015: contributor

The only existing non-consensus module that is shared between wallet and main, and knows about transactions, is validationinterface.
zkbot referenced this in commit 57d420e2f8 on Feb 15, 2017
zkbot referenced this in commit 88c209dba6 on Feb 20, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5881 Fix and improve txn_doublespend.py test