Fix bug #4355 #4411

pull haight6716 wants to merge 2 commits into bitcoin:master from haight6716:master changing 1 files +1 −3

haight6716 commented at 7:27 AM on June 25, 2014: contributor

No longer start up in testnet mode when presented with a BIP 72 link that has no fallback address.
laanwj commented at 7:42 AM on June 25, 2014: member

Haven't tested but from my understanding this would no longer redirect legacy bitcoin: URIs with a testnet address to a testnet instance.

A better solution would be to skip these heuristics entirely if the address is empty.
fix bug #4355 4210835f2f
haight6716 commented at 6:10 PM on June 25, 2014: contributor

Thanks @laanwj, check out the latest (4210835) which checks for empty string specifically, prior to parsing the address. Tested with:

$ ./bitcoin-qt 'bitcoin:?r=http://www.example.com/' .. fixed the original problem - this launches mainnet.

$ ./bitcoin-qt bitcoin:mngeNQbTKnmaMbx8EXCYdwUbnt9JJD52cC .. launches testnet

$ ./bitcoin-qt 'bitcoin:1NXXeQRyMFFFRfyUix2o7mk1vhvk2Nxp78' .. sanity check - launches mainnet.

Sorry for the back & forth - that merge conflict message is from an earlier attempt. I'm just getting up to speed on doing proper pull requests plus had the wrong github user id set. 4210835 should apply cleanly.
fix bug #4355, try again bffc63073a
in src/qt/paymentserver.cpp:None in 4210835f2f outdated
```
 194 | @@ -195,11 +195,10 @@ bool PaymentServer::ipcParseCommandLine(int argc, char* argv[])
 195 |              savedPaymentRequests.append(arg);
 196 |  
 197 |              SendCoinsRecipient r;
 198 | -            if (GUIUtil::parseBitcoinURI(arg, &r))
 199 | +            SelectParams(CChainParams::MAIN);
```
laanwj commented at 1:20 PM on June 26, 2014:

Now you're forcing the network always to main, even if someone specifies -testnet :) Please keep this line inside the if().

haight6716 commented at 4:30 PM on July 1, 2014:

In my tests, -testnet still worked as expected, but I updated it as you say.
BitcoinPullTester commented at 4:27 PM on July 1, 2014: none

Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/p4411_bffc63073a7b74b4672dddb145cc5956ad0079fa/ for test log.

This pull does not merge cleanly onto current master This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
laanwj closed this on Jul 2, 2014
laanwj commented at 7:48 AM on July 2, 2014: member

Merged via dd49e92
laanwj referenced this in commit bf75a3d3fd on Aug 18, 2014
MathyV referenced this in commit e779613ad2 on Nov 3, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors

haight6716

laanwj

BitcoinPullTester