wallet: Detect `DuplicateAddress` earlier #610

This PR makes check for DuplicateAddress error easier and earlier.

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.

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?

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: https://github.com/furszy/bitcoin/commit/6115d09946b2fa3b25d005f748a5f9a0992c973b . 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.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

Going to leave this PR up for grabs.

cc @furszy