Differential D614

MalFix: Type separation of txids to txhash and unspentid
AbandonedPublic
Actions

Authored by tomtomtom7 on Oct 18 2017, 08:18.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

Unspentid is used as a temporary term to allow properly reviewing the split.
This will be refactored back to txid.

The purpose of the split is to prepare for changing unspentid
to be calculated without signatures

Separate uint256 hashes in types txhash_t and unspentid_t
Add index to the mempool to allow lookup by unspentid
Replace Outpoint txid field with unspentid

The separation is only through types. Unspentids are still calculated
in the same way as txids. No functional changes.

Test Plan

This is an intermediate refactoring diff. Tests don't pass until next diffs.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

arcpatch-D597

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1008
Build 1008: arc lint + arc unit

Event Timeline

tomtomtom7 created this revision.Oct 18 2017, 08:18

Owners added a reviewer: Restricted Owners Package.Oct 18 2017, 08:18

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 18 2017, 08:18

This is essentially unreviewable. It's huge, touches everything and does many things at once.

Here is how you go about doing such refactorings:

Introduce types for txhash and txid . Ensure they both have a different type, and implicitly convert to uint256.
move subsystems, one by one, to use either ones of these types.
When all subsystem are migrated, start introducing feature that rely on the fact that the txhash and txid are different.

This revision now requires changes to proceed.Oct 18 2017, 14:31

Thanks. I understand this is too big as I commented on slack, but I also think it's wrong and needs discussion.

Changing txid => txhash is not only a very large operation but also strange as we cannot use our new txid to identify transactions. It's a misnomer.

For instance, you can't do a gettransaction with a txid or use it for other rpc/rest methods, use a txid in the wallet display, or (as discusses) use it for normal P2P traffic.

Why do we want this? If we follow MalFix and only change the prev_tx_out then IMO we are much better of with a new name for this new field.

tomtomtom7 abandoned this revision.Dec 7 2017, 09:27

tomtomtom7 mentioned this in D665: Start refactoring txid -> txhash.

Revision Contents
Changeset List

Path

Size

src/

bench/

ccoins_caching.cpp

6 lines

mempool_eviction.cpp

12 lines

31 lines

blockencodings.h

52 lines

blockencodings.cpp

76 lines

8 lines

2 lines

5 lines

23 lines

consensus/

6 lines

4 lines

24 lines

merkleblock.cpp

46 lines

8 lines

8 lines

net_processing.cpp

94 lines

policy/

2 lines

12 lines

primitives/

66 lines

transaction.cpp

39 lines

6 lines

qt/

bitcoinstrings.cpp

2 lines

coincontroldialog.cpp

28 lines

transactiondesc.cpp

2 lines

transactiondescdialog.cpp

2 lines

transactionrecord.h

9 lines

transactionrecord.cpp

4 lines

transactiontablemodel.h

2 lines

transactiontablemodel.cpp

30 lines

transactionview.h

2 lines

transactionview.cpp

17 lines

6 lines

walletmodel.cpp

41 lines

15 lines

rpc/

103 lines

2 lines

29 lines

rawtransaction.cpp

166 lines

test/

6 lines

bitcoin-util-test.py

8 lines

blockcheck_tests.cpp

6 lines

blockencodings_tests.cpp

83 lines

bloom_tests.cpp

62 lines

coins_tests.cpp

36 lines

mempool_tests.cpp

232 lines

merkle_tests.cpp

4 lines

miner_tests.cpp

130 lines

multisig_tests.cpp

4 lines

33 lines

policyestimator_tests.cpp

34 lines

16 lines

script_P2SH_tests.cpp

13 lines

script_antireplay_tests.cpp

2 lines

script_tests.cpp

2 lines

sighash_tests.cpp

2 lines

sigopcount_tests.cpp

4 lines

transaction_tests.cpp

14 lines

txvalidationcache_tests.cpp

15 lines

21 lines

4 lines

14 lines

91 lines

139 lines

3 lines

2 lines

98 lines

validationinterface.h

5 lines

validationinterface.cpp

1 line

wallet/

17 lines

159 lines

test/

accounting_tests.cpp

6 lines

48 lines

360 lines

6 lines

28 lines

zmq/

zmqpublishnotifier.cpp

10 lines

Diff 1580

src/bench/ccoins_caching.cpp

Loading...

src/bench/mempool_eviction.cpp

Loading...

src/bitcoin-tx.cpp

Loading...

src/blockencodings.h

Loading...

src/blockencodings.cpp

Loading...

src/bloom.cpp

Loading...

src/chain.h

Loading...

src/coins.h

Loading...

src/coins.cpp

Loading...

src/consensus/merkle.cpp

Loading...

src/core_write.cpp

Loading...

src/merkleblock.h

Loading...

src/merkleblock.cpp

Loading...

src/miner.cpp

Loading...

src/net.h

Loading...

src/net_processing.cpp

Loading...

src/policy/fees.h

Loading...

src/policy/fees.cpp

Loading...

src/primitives/transaction.h

Loading...

src/primitives/transaction.cpp

Loading...

src/protocol.h

Loading...

src/qt/bitcoinstrings.cpp

Loading...

src/qt/coincontroldialog.cpp

Loading...

src/qt/transactiondesc.cpp

Loading...

src/qt/transactiondescdialog.cpp

Loading...

src/qt/transactionrecord.h

Loading...

src/qt/transactionrecord.cpp

Loading...

src/qt/transactiontablemodel.h

Loading...

src/qt/transactiontablemodel.cpp

Loading...

src/qt/transactionview.h

Loading...

src/qt/transactionview.cpp

Loading...

src/qt/walletmodel.h

Loading...

src/qt/walletmodel.cpp

Loading...

src/rest.cpp

Loading...

src/rpc/blockchain.cpp

Loading...

src/rpc/client.cpp

Loading...

src/rpc/mining.cpp

Loading...

src/rpc/rawtransaction.cpp

Loading...

src/test/DoS_tests.cpp

Loading...

src/test/bitcoin-util-test.py

Loading...

src/test/blockcheck_tests.cpp

Loading...

src/test/blockencodings_tests.cpp

Loading...

src/test/bloom_tests.cpp

Loading...

src/test/coins_tests.cpp

Loading...

src/test/mempool_tests.cpp

Loading...

src/test/merkle_tests.cpp

Loading...

src/test/miner_tests.cpp

Loading...

src/test/multisig_tests.cpp

Loading...

src/test/pmt_tests.cpp

Loading...

src/test/policyestimator_tests.cpp

Loading...

src/test/rpc_tests.cpp

Loading...

src/test/script_P2SH_tests.cpp

Loading...

src/test/script_antireplay_tests.cpp

Loading...

src/test/script_tests.cpp

Loading...

src/test/sighash_tests.cpp

Loading...

src/test/sigopcount_tests.cpp

Loading...

src/test/transaction_tests.cpp

Loading...

src/test/txvalidationcache_tests.cpp

Loading...

src/test/undo_tests.cpp

Loading...

src/txdb.h

Loading...

src/txdb.cpp

Loading...

src/txmempool.h

Loading...

src/txmempool.cpp

Loading...

src/undo.h

Loading...

src/validation.h

Loading...

src/validation.cpp

Loading...

src/validationinterface.h

Loading...

src/validationinterface.cpp

Loading...

src/wallet/rpcdump.cpp

Loading...

src/wallet/rpcwallet.cpp

Loading...

src/wallet/test/accounting_tests.cpp

Loading...

src/wallet/wallet.h

Loading...

src/wallet/wallet.cpp

Loading...

src/wallet/walletdb.h

Loading...

src/wallet/walletdb.cpp

Loading...

src/zmq/zmqpublishnotifier.cpp

Loading...