qt: Remove network detection based on address in BIP21

[laanwj]

This is removes some ugly and brittle code that switches the global network to testnet based on a provided address. I think in practice it's very unlikely for testnet BIP21 payment URIs to be used, and if so it's for testing so it's easy enough to manually copy it. Or to specify -testnet explicitly.

There is already no such case for -regtest or -signet.

After this change it will only accept addresses for the explicitly selected network. Others will result in a "wrong network" popup.

There is also a possibility for refactor after this as the initialization order of PaymentServer::ipcParseCommandLine isn't important anymore (well, it still has to be before PaymentServer::ipcSendCommandLine, maybe even merged with it), but I have not done so here.

[laanwj]

Discussion on IRC that triggered this change:

2022-03-08 18:07:45     dongcarl        Really sus-ed out that paymentserver can call SelectParams and cha
nge the global params... That's due to be removed right?
2022-03-08 19:32:46     laanwj  oh you mean selecting which set of parameters to use based on the provided bitcoin URI 
2022-03-08 19:33:56     laanwj  to be honest i'm not sure how important that is, who is using testnet bitcoin URIs in a browser or such
2022-03-08 19:34:09     laanwj  i'd personally be fine with removing that
2022-03-08 19:36:58     laanwj  i guess you could still explicitly pass -testnet anyway, instead of making it guess from the address?
2022-03-08 19:37:52     laanwj  the comment is also outdated

[fanquake]

cc @dongcarl

[jonatack]

Code review ACK 2d41f4d

There is also a possibility for refactor after this as the initialization order of PaymentServer::ipcParseCommandLine isn't important anymore

Was curious about this and 2102ab9 looks like the related commit.

The comment just above in this function looks like it should be moved as well.

// Warning: ipcSendCommandLine() is called early in init,
// so don't use "Q_EMIT message()", but "QMessageBox::"!

[laanwj]

The comment just above in this function looks like it should be moved as well.

I'm not sure. Does the comment refer to the wrong function, or should it be moved? I think it's valid for both ipcParseCommandLine and ipcSendCommandLine. That said, neither of them actually ever needs to display a warning/error.

I could update the comment but also remove it.

[jonatack]

I could update the comment but also remove it.

Oh sorry, I was thinking more when refactoring as you mentioned; it's not related to this change.

[jarolrod]

crACK 2d41f4d

[kristapsk]

ACK 2d41f4d

[laanwj]

I hadn't noticed before that savedPaymentRequests is a static QSet<QString>. Will remove the explicit duplicate detection, which is no longer necessary without the other logic, and push again (sorry).

[jonatack]

Good point. Verified at https://doc.qt.io/qt-5/qset.html#insert that a QSet behaves (as expected) like a set: "Inserts item value into the set, if value isn't already in the set."

ACK b7dbc83

[achow101]

ACK b7dbc83