wallet: Detect DuplicateAddress earlier

[hebasto]

This PR makes check for DuplicateAddress error easier and earlier.

[furszy]

Could move it even earlier in the flow and remove the code from the model (which requires the wallet to be locked):

Inside SendCoinsDialog ::PrepareSendText, before calling WalletModel::prepareTransaction, we are looping over all the entries to load up the recipients list --> Line 257 of SendCoinsDialog.

[hebasto]

which requires the wallet to be locked

Do you mean "unlocking" using WalletModel::UnlockContext?

Inside SendCoinsDialog ::PrepareSendText, before calling WalletModel::prepareTransaction, we are looping over all the entries to load up the recipients list --> Line 257 of SendCoinsDialog.

Would it make logic a bit messier?

[furszy]

Do you mean "unlocking" using WalletModel::UnlockContext?

Yeah. I skipped the first two letters.

Would it make logic a bit messier?

I think that the current logic is messy actually. We are already validating all the recipients' data there. Check this out: furszy/bitcoin@6115d09 . I made the code flow cleaner there.

Before creating the recipient and adding it to the temporal recipients vector, we call to SendCoinsEntry::validate, which validates the address and the amount (failing if the address is invalid, or the amount is 0 or dust). So, when we are inside WalletModel:: prepareTransaction, we already know whether the address and/or the amount are valid or not. Those checks are duplicated.

I reworked the area a bit here (stopped after few commits to not enter in a long rabbit hole):
https://github.com/furszy/bitcoin/commits/220529-dupaddr

Feel free to cherry-pick any of those commits if you like them, all yours :). Maybe we could team up to get deeper over this rabbit hole in follow-up PRs.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

[hebasto]

Going to leave this PR up for grabs.

cc @furszy