This makes the client support parsing cashaddr encoded addresses for all networks.
Depends on D797
Details
Details
- Reviewers
dagurval schancel - Group Reviewers
Restricted Project - Commits
- rSTAGING2966060caa70: Support parsing cashaddr from command line
rABC2966060caa70: Support parsing cashaddr from command line
./bitcoin-qt "bitcoincash:mz8DUu5atAfNEBNeQHrpdGkfWZ9vmk3Q13?amount=42.00000000&label=test1&message=a%20message"
Ensure that the client boot in testnet mode and the proper sent is prefilled.
./bitcoin-qt "bitcoincash:1F6MqpCw3f8JRErcZMYjkFLBzpFvvjobZZ?amount=42.00000000&label=mainnet1&message=a%20message"
Ensure the client boots in mainnet and the sent form is prefilled.
./bitcoin-qt "bchtest:qp5dzwdct8xcpec62z799rdfpylxftsl6v0trfz2m9?amount=42.00000000&label=cashtest2&message=a%20message" "bitcoincash:mz8DUu5atAfNEBNeQHrpdGkfWZ9vmk3Q13?amount=42.00000000&label=test1&message=a%20message"
Ensure that the client start on testnet and fill up the sent form properly.
./bitcoin-qt "bitcoincash:qrc5xvnqwv3esj8hs5w85gh3xjwgnnjfvv3ll6wmdu?amount=42.00000000&label=test3&message=a%20message" "bitcoincash:1F6MqpCw3f8JRErcZMYjkFLBzpFvvjobZZ?amount=42.00000000&label=mainnet1&message=a%20message"
dito
./bitcoin-qt "bchreg:qrxvv8fpv42wvlls6z4mp9qvrs3kar2pau5tssl8xd?amount=42.00000000&label=cashtest1&message=a%20message"
Start in regtest mode.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- arcpatch-D583
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1397 Build 1397: arc lint + arc unit
Event Timeline
Comment Actions
Test plan is not adequate. Manual is not something I can reproduce if I wanted/needed to.
It also looks like this diff does several things at once. The GUIUtil::parseBitcoinURI a brain dead change that could be in by now, and what's not in there could be reviewed much more easily.
| src/qt/guiutil.cpp | ||
|---|---|---|
| 169 ↗ | (On Diff #1870) | Braces |
| src/qt/paymentserver.cpp | ||
| 203 ↗ | (On Diff #1870) | remove |
| 210 ↗ | (On Diff #1870) | You have created an helper function to do this. |
| 227 ↗ | (On Diff #1870) | This is validating that it is a valid destination. |
| 234 ↗ | (On Diff #1870) | This is not. |
| 395 ↗ | (On Diff #1870) | You have an helper function for this one as well. |
Comment Actions
Test plan is not adequate. Manual is not something I can reproduce if I wanted/needed to.
How can the test plan be improved? Should it include the manual test steps? I'm afraid this cannot be automated in any sane way.
| src/qt/paymentserver.cpp | ||
|---|---|---|
| 210 ↗ | (On Diff #1870) | The helper function requires a config object, which we don't have. The alternative is to create a dummy config object. |
| 227 ↗ | (On Diff #1870) | My interpretation of this code is that it's a hack to check if it's a mainnet address or a testnet address. Validation is a side effect of that. We can do that now by looking at the URI scheme. Validation is done in handleURIOrFile. I can add validation though before using the chain params. |
| 395 ↗ | (On Diff #1870) | Hard to tell if the GetConfig() object is ready here (or will be after some change in the future). |
Comment Actions
This is 99% there.
| src/qt/paymentserver.cpp | ||
|---|---|---|
| 242 ↗ | (On Diff #2085) | Putting that here makes it is very dangerous function to use in general. I would extract the loop and the SelectParams call in ipcParseCommandLine and keep the inner working of the loop in that function, which would now take networks as an argument. |
| src/qt/paymentserver.cpp | ||
|---|---|---|
| 229 ↗ | (On Diff #2137) | It doesn't seem like you need to return a pair. |
| 266 ↗ | (On Diff #2137) | I guess this guy should check if the network provided int he address match or not. There are clearly different source of network here and they should all be consistent. We don't want to be sending coins on the wrong network. |
| src/qt/paymentserver.cpp | ||
|---|---|---|
| 505 | The prefix is not the same when not on mainnet. | |