fix: validate addresses during tx broadcast

[woodenfurniture]

Description

Adds validation to TXs during broadcast. Includes in-memory caching and request deduplication to minimise impact of rate limits. Also removes initial implementation address validation.

Pull Request Type

• 🐛 Bug fix (Non-breaking Change: Fixes an issue)
• 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
• 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

closes #5438

Risk

Risk of broken trades, staking, sends etc. Affects all chains.

Testing

Check transactions are correctly transmitted across the app.

Engineering

Operations

Screenshots (if applicable)

Successful trade with single deduped validation request, not validating the contract address as though it's a receiver

Successful trade for manual receive address, 2 validation requests - 1 for sender, 1 for receiver

[gomesalexandre]

A few comments/q. but conceptual stamp pending functional review

[gomesalexandre]

First pass, tested locally with native.

EVM token self-send

Since EVM chains are account-based and the sender and receiver are the same, I believe we shouldn't be validating here in case of self-send?

EVM native asset self-send

Since EVM chains are account-based and the sender and receiver are the same, I believe we shouldn't be validating here in case of self-send?

UTXO self-send

Note, we're currently validating the send address using the xpub which we can't do for privacy reasons (and wouldn't validate anything):

Same-chain trade - confirmed the XHR isn't made to validate the address

Cross-chain trade

Trade went through, however, only the sender address was checked against (the first request is a preflight OPTIONS), not the receiver address.

ATOM self-send

Since ATOM is account-based and the sender and receiver are the same, I believe we shouldn't be validating here in case of self-send?

[gomesalexandre]

Retested locally:

• Self-send - UTXOs use the receiver address as an address to verify (not the xpub)

• Self-send - UTXOs use the sender address as an address to verify (not the xpub)

• EVM self-send - address is verified on the first check, and is only verified on the sender side (since it's already cached by the time we get to the receiver side branch)

• EVM trades - address is verified (again, only once since it only needs to be on one side)

• EVM -> UTXO THOR trade

• UTXO -> UTXO THOR trade

[kaladinlight]

First thought is that build transaction is a better place to put this logic as we have most of the data available here to do the address validation. For cosmossdk we have all of the data available we are using to build the messages and wouldn't require updating the chain adapter api. For utxos we would have access to all of the inputs being used to build the send transaction (we aren't currently validating all of the input addresses which we should be doing). For evms it is a bit more difficult due to the input data embedding addresses though. It would be easy to handle the standard send transaction, but the build custom tx would probably still need addresses passed in (specifically for the swap transactions).

Main thing missing is the utxo input/output address validation. The other thought is just if we could do this without changing the chain adapter api or at least isolate it to buildCustomTx. Thoughts?

[woodenfurniture]

First thought is that build transaction is a better place to put this logic as we have most of the data available here to do the address validation. For cosmossdk we have all of the data available we are using to build the messages and wouldn't require updating the chain adapter api. For utxos we would have access to all of the inputs being used to build the send transaction (we aren't currently validating all of the input addresses which we should be doing). For evms it is a bit more difficult due to the input data embedding addresses though. It would be easy to handle the standard send transaction, but the build custom tx would probably still need addresses passed in (specifically for the swap transactions).

Main thing missing is the utxo input/output address validation. The other thought is just if we could do this without changing the chain adapter api or at least isolate it to buildCustomTx. Thoughts?

100% agree, will action in follow-up in the spirit of getting this partially implemented sooner