⚙ D8692 [CashAddr] replace bitcoincash: prefix with ecash: in the node

majcosta created this revision.Dec 17 2020, 20:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 17 2020, 20:14

majcosta requested review of this revision.Dec 17 2020, 20:14

majcosta mentioned this in D8676: [CashAddr] set each network CashAddr prefixes in chainparams.cpp.Dec 17 2020, 20:15

majcosta edited the summary of this revision. (Show Details)

majcosta retitled this revision from [CashAddr] set each network CashAddr prefixes in chainparams.cpp to [CashAddr] replace bitcoincash: prefix with abc:.Dec 17 2020, 20:18

majcosta retitled this revision from [CashAddr] replace bitcoincash: prefix with abc: to [CashAddr] replace bitcoincash: prefix with abc: in the node.

Harbormaster completed remote builds in B14437: Diff 26417.Dec 17 2020, 20:38

jasonbcox requested changes to this revision.Dec 17 2020, 22:36

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

src/qt/test/bitcoinaddressvalidatortests.cpp
26 ↗	(On Diff #26417)	The comment is now wrong for this case. Either this case is new and deserves its own check or the case is wrong and was changed for some reason.
src/test/cashaddrenc_tests.cpp
72 ↗	(On Diff #26417)	Use CASHADDR_PREFIX

This revision now requires changes to proceed.Dec 17 2020, 22:36

jasonbcox added inline comments.Dec 17 2020, 23:54

test/functional/abc_mining_basic.py
25 ↗	(On Diff #26417)	As we discussed offline, this and similar codepaths related to mining will need to be backwards-compatible with the `bitcoincash:` prefix.

addressed comments

Harbormaster completed remote builds in B14453: Diff 26440.Dec 18 2020, 20:07

Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

[372/431] Running utility command for check-bitcoin-sync_tests
[373/431] Running utility command for check-bitcoin-sigencoding_tests
[374/431] Running utility command for check-bitcoin-sighashtype_tests
[375/431] Running utility command for check-bitcoin-bip32_tests
[376/431] bitcoin: testing torcontrol_tests
[377/431] Running utility command for check-bitcoin-torcontrol_tests
[378/431] bitcoin: testing timedata_tests
[379/431] bitcoin: testing uint256_tests
[380/431] bitcoin: testing streams_tests
[381/431] bitcoin: testing undo_tests
[382/431] bitcoin: testing wallet_crypto_tests
[383/431] Running utility command for check-bitcoin-timedata_tests
[384/431] Running utility command for check-bitcoin-uint256_tests
[385/431] Running utility command for check-bitcoin-streams_tests
[386/431] Running utility command for check-bitcoin-undo_tests
[387/431] Running utility command for check-bitcoin-wallet_crypto_tests
[388/431] bitcoin: testing util_threadnames_tests
[389/431] bitcoin: testing compilerbug_tests
[390/431] bitcoin: testing validation_chainstatemanager_tests
[391/431] Running utility command for check-bitcoin-util_threadnames_tests
[392/431] bitcoin: testing checkpoints_tests
[393/431] Running utility command for check-bitcoin-compilerbug_tests
[394/431] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[395/431] Running utility command for check-bitcoin-checkpoints_tests
[396/431] bitcoin: testing txvalidationcache_tests
[397/431] bitcoin: testing cuckoocache_tests
[398/431] Running utility command for check-bitcoin-txvalidationcache_tests
[399/431] Running utility command for check-bitcoin-cuckoocache_tests
[400/431] bitcoin: testing validationinterface_tests
[401/431] Running utility command for check-bitcoin-validationinterface_tests
[402/431] bitcoin: testing radix_tests
[403/431] Running utility command for check-bitcoin-radix_tests
[404/431] bitcoin: testing getarg_tests
[405/431] bitcoin: testing ref_tests
[406/431] Running utility command for check-bitcoin-getarg_tests
[407/431] Running utility command for check-bitcoin-ref_tests
[408/431] bitcoin: testing crypto_tests
[409/431] Running utility command for check-bitcoin-crypto_tests
[410/431] bitcoin: testing blockcheck_tests
[411/431] Running utility command for check-bitcoin-blockcheck_tests
[412/431] bitcoin: testing validation_block_tests
[413/431] bitcoin: testing monolith_opcodes_tests
[414/431] bitcoin: testing validation_tests
[415/431] Running utility command for check-bitcoin-validation_block_tests
[416/431] Running utility command for check-bitcoin-monolith_opcodes_tests
[417/431] Running utility command for check-bitcoin-validation_tests
[418/431] bitcoin: testing script_tests
[419/431] Running utility command for check-bitcoin-script_tests
[420/431] bitcoin: testing skiplist_tests
[421/431] Running utility command for check-bitcoin-skiplist_tests
[422/431] bitcoin: testing util_tests
[423/431] Running utility command for check-bitcoin-util_tests
[424/431] bitcoin: testing op_reversebytes_tests
[425/431] Running utility command for check-bitcoin-op_reversebytes_tests
[426/431] bitcoin: testing transaction_tests
[427/431] Running utility command for check-bitcoin-transaction_tests
[428/431] bitcoin: testing coins_tests
[429/431] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

made 'bitcoincash:' prefix backward compatible for the time being and duplicated a few tests for it

majcosta edited the summary of this revision. (Show Details)Dec 21 2020, 18:24

Harbormaster completed remote builds in B14492: Diff 26507.Dec 21 2020, 18:26

abc_mining_basic.py still needs to test that the old prefix is valid for mining

src/test/cashaddrenc_tests.cpp
303 ↗	(On Diff #26507)	reusing variables in tests is generally a bad idea

This revision now requires changes to proceed.Dec 21 2020, 19:25

addressed comment on reuse of test variables and added a test to ensure miner fund addresses with both supported prefixes behave the same

majcosta planned changes to this revision.Dec 21 2020, 20:52

Harbormaster completed remote builds in B14494: Diff 26512.Dec 21 2020, 20:56

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_feature_minerfund.py ======

------- Stdout: -------
2020-12-21T20:54:56.772000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201221_205348/abc_feature_minerfund_238
2020-12-21T20:54:57.047000Z TestFramework (INFO): Create some history
2020-12-21T20:54:57.128000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 97, in run_test
    MINER_FUND_ADDR)
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not(abcreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgyrm5ege8 == bchreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgd35g0pkl)
2020-12-21T20:54:57.180000Z TestFramework (INFO): Stopping nodes
2020-12-21T20:54:57.482000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201221_205348/abc_feature_minerfund_238
2020-12-21T20:54:57.482000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201221_205348/abc_feature_minerfund_238/test_framework.log
2020-12-21T20:54:57.482000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20201221_205348/abc_feature_minerfund_238' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_feature_minerfund.py

fixed misnamed variable on abc_feature_minerfund.py

Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_feature_minerfund.py ======

------- Stdout: -------
2020-12-21T20:58:50.853000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20201221_205517/abc_feature_minerfund_561
2020-12-21T20:58:51.117000Z TestFramework (INFO): Create some history
2020-12-21T20:58:51.238000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 97, in run_test
    MINER_FUND_ADDR)
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not(abcreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgyrm5ege8 == bchreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgd35g0pkl)
2020-12-21T20:58:51.289000Z TestFramework (INFO): Stopping nodes
2020-12-21T20:58:51.592000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20201221_205517/abc_feature_minerfund_561
2020-12-21T20:58:51.592000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20201221_205517/abc_feature_minerfund_561/test_framework.log
2020-12-21T20:58:51.592000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20201221_205517/abc_feature_minerfund_561' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_feature_minerfund.py

Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_feature_minerfund.py ======

------- Stdout: -------
2020-12-21T20:59:33.891000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205648/abc_feature_minerfund_685
2020-12-21T20:59:34.151000Z TestFramework (INFO): Create some history
2020-12-21T20:59:34.190000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 97, in run_test
    MINER_FUND_ADDR)
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not(abcreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgyrm5ege8 == bchreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgd35g0pkl)
2020-12-21T20:59:34.241000Z TestFramework (INFO): Stopping nodes
2020-12-21T20:59:34.443000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205648/abc_feature_minerfund_685
2020-12-21T20:59:34.443000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205648/abc_feature_minerfund_685/test_framework.log
2020-12-21T20:59:34.443000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205648/abc_feature_minerfund_685' to consolidate all logs
====== Bitcoin ABC functional tests with the next upgrade activated: abc_feature_minerfund.py ======

------- Stdout: -------
2020-12-21T21:02:18.288000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205943/abc_feature_minerfund_275
2020-12-21T21:02:18.571000Z TestFramework (INFO): Create some history
2020-12-21T21:02:18.643000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 120, in main
    self.run_test()
  File "/work/test/functional/abc_feature_minerfund.py", line 97, in run_test
    MINER_FUND_ADDR)
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not(abcreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgyrm5ege8 == bchreg:pqnqv9lt7e5vjyp0w88zf2af0l92l8rxdgd35g0pkl)
2020-12-21T21:02:18.694000Z TestFramework (INFO): Stopping nodes
2020-12-21T21:02:18.945000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205943/abc_feature_minerfund_275
2020-12-21T21:02:18.946000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205943/abc_feature_minerfund_275/test_framework.log
2020-12-21T21:02:18.946000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20201221_205943/abc_feature_minerfund_275' to consolidate all logs

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_feature_minerfund.py
Bitcoin ABC functional tests with the next upgrade activated: abc_feature_minerfund.py

Harbormaster completed remote builds in B14495: Diff 26513.Dec 21 2020, 21:12

Tests still failing.

This revision now requires changes to proceed.Dec 28 2020, 17:49

rebase

Harbormaster completed remote builds in B14531: Diff 26576.Dec 28 2020, 20:45

jasonbcox requested changes to this revision.Dec 29 2020, 17:21

jasonbcox added inline comments.

test/functional/abc_feature_minerfund.py
24 ↗	(On Diff #26576)	Nit: Call the old one "legacy" and remove "new" from the current one. This is because we should expect to remove the legacy option at some point, but we don't want to call ABC new going forward. This way you don't put off another refactor.
40 ↗	(On Diff #26576)	why create new variables here?

This revision now requires changes to proceed.Dec 29 2020, 17:21

yea it does look nicer that way, thanks

Harbormaster completed remote builds in B14533: Diff 26582.Dec 29 2020, 20:21

this patch needs to be several patches.

src/cashaddrenc.cpp
108 ↗	(On Diff #26582)	The old and famous if/return:else pattern make a wild appearance.
src/minerfund.cpp
29 ↗	(On Diff #26582)	If there is a backward compatible path, then this belongs in another diff.
src/network.h
19 ↗	(On Diff #26582)	Now that we do not support BCHN, what purpose does this serve?
test/functional/abc_feature_minerfund.py
24 ↗	(On Diff #26582)	This does not belong here.

This revision now requires changes to proceed.Jan 2 2021, 20:38

cut all backwards compatibility changes from the patch, this is a pure prefix change now.

missed some stuff in tests

Harbormaster completed remote builds in B14920: Diff 27211.Jan 21 2021, 21:34

removed the last backward compatibility tests I missed the first time around.

note: I left in place the tests that hardcode the prefix in cashaddrenc_tests.cpp, for historical reasons and because they don't rely on what the node considers to be the default prefix for a particular chain.

Harbormaster completed remote builds in B14921: Diff 27212.Jan 21 2021, 21:50

majcosta added a child revision: D9020: [CashAddr] deprecate legacy prefixes maintaining backwards compatibility.Jan 21 2021, 21:52

rebase

Harbormaster completed remote builds in B14947: Diff 27256.Jan 22 2021, 11:57

*ecash

majcosta retitled this revision from [CashAddr] replace bitcoincash: prefix with abc: in the node to [CashAddr] replace bitcoincash: prefix with ecash: in the node.Thu, Feb 4, 21:37

majcosta edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B15132: Diff 27568.Thu, Feb 4, 21:50

jasonbcox resigned from this revision.Fri, Feb 5, 20:04

majcosta edited the summary of this revision. (Show Details)Sat, Feb 6, 01:31

[CashAddr] replace bitcoincash: prefix with ecash: in the node
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 27568

src/chainparams.cpp

src/minerfund.cpp

src/qt/test/bitcoinaddressvalidatortests.cpp

src/qt/test/guiutiltests.cpp

src/qt/test/uritests.cpp

src/qt/test/wallettests.cpp

src/test/cashaddrenc_tests.cpp

src/test/dstencode_tests.cpp

test/functional/abc_feature_minerfund.py

test/functional/abc_mining_basic.py

test/functional/abc_p2p_avalanche.py

test/functional/data/rpc_bip67.json

test/functional/data/rpc_psbt.json

test/functional/rpc_deriveaddresses.py

test/functional/rpc_generateblock.py

test/functional/test_framework/address.py

test/functional/wallet_basic.py

test/functional/wallet_importdescriptors.py

test/functional/wallet_importmulti.py

[CashAddr] replace bitcoincash: prefix with ecash: in the nodeNeeds ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 27568

src/chainparams.cpp

src/minerfund.cpp

src/qt/test/bitcoinaddressvalidatortests.cpp

src/qt/test/guiutiltests.cpp

src/qt/test/uritests.cpp

src/qt/test/wallettests.cpp

src/test/cashaddrenc_tests.cpp

src/test/dstencode_tests.cpp

test/functional/abc_feature_minerfund.py

test/functional/abc_mining_basic.py

test/functional/abc_p2p_avalanche.py

test/functional/data/rpc_bip67.json

test/functional/data/rpc_psbt.json

test/functional/rpc_deriveaddresses.py

test/functional/rpc_generateblock.py

test/functional/test_framework/address.py

test/functional/wallet_basic.py

test/functional/wallet_importdescriptors.py

test/functional/wallet_importmulti.py

[CashAddr] replace bitcoincash: prefix with ecash: in the node
Needs ReviewPublic
Actions

Revision Contents
Changeset List