Hardened wallet actions #3723
Merged
Hardened wallet actions #3723
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current version of the main UTXO lookup mechanism looks just on the last five confirmed transactions targeting the wallet public key hash to determine the plain text main UTXO of the wallet. This mechanism is not ideal as it doesn't recognize the difference between true wallet transactions and arbitrary transfers to the wallet public key hash that can be made by anyone. That means it is enough to craft several arbitrary transfers to block the main UTXO lookup and prevent the given wallet to perform actions requested by the wallet coordinator. To address that problem, we are improving the mechanism to take the full transaction history into account. To make it efficient, we are taking just transaction hashes first and fetch full transaction data only for the latest transactions, where the chance to find the wallet UTXO is the highest.
The goal of this mechanism is to ensure the previous wallet transaction on Bitcoin chain was properly proved to the Bridge contract. This must be ensured before initiating new wallet transactions in order to maintain proper Bitcoin transaction ordering enforced by the Bridge contract. The current version of this mechanism was a naive implementation that checked whether the wallet main UTXO comes from the latest Bitcoin transaction or, if there was no main UTXO, the wallet doesn't have a transaction history. Additionally, this implementation required the mempool to be empty for both cases. This logic is prone to spam transactions sending funds to wallet addresses which cause the wallet to abandon all actions proposed by the coordinator. Here we fix that by using a more sophisticated mechanism: For wallets having a registered main UTXO, it is enough to check whether their registered UTXO is still among the confirmed unspent outputs from the Bitcoin network standpoint. In order to do that check, we are leveraging ElectrumX `listunspent` method that returns outputs not used as inputs by any (either confirmed or mempool) transaction. If a wallet uses their main UTXO to produce another transaction, `listunspent` will not show it and `EnsureWalletSyncedBetweenChain` will detect this state drift preventing to start another action. For fresh wallets which don't have main UTXO yet, the situation is more complicated. In that case, we are additionally taking mempool UTXOs into account. If there are no UTXOs at all, that implies the wallet has not produced any (either confirmed or mempool) Bitcoin transaction so far. If some UTXOs targets the wallet, we need to check whether they are spam or actually result of proper wallet transaction. We do this by checking the first input of each transaction. Very first transactions of wallets are always deposit sweeps and all their inputs must point to revealed deposit. If the first input refers to a deposit in that case, that means the wallet already produced their first transaction on Bitcoin and no other action should be taken until the corresponding SPV proof is submitted to the Bridge. Otherwise, such a transaction is spam. If all transactions are spam, the wallet can safely start the given action.
tomaszslabon
reviewed
Oct 2, 2023
| } | ||
| allUtxos := append(confirmedUtxos, mempoolUtxos...) | ||
| if len(allUtxos) == 0 { | ||
| // Wallet have not produced any transactions - we are good. |
There was a problem hiding this comment.
Suggested change
| // Wallet have not produced any transactions - we are good. | |
| // Wallet has not produced any transactions - we are good. |
tomaszslabon
reviewed
Oct 2, 2023
| } | ||
| for _, utxo := range allUtxos { | ||
| // We know that valid first transaction of the wallet always | ||
| // have just one output. Any utxos with output index other |
There was a problem hiding this comment.
Suggested change
| // have just one output. Any utxos with output index other | |
| // has just one output. Any utxos with output index other |
tomaszslabon
approved these changes
Oct 2, 2023
tomaszslabon
reviewed
Oct 2, 2023
Comment on lines
+525
to
+526
| // currently doing their first Bitcoin transaction but, in the same | ||
| // time, we cannot just assume their transaction history must be |
There was a problem hiding this comment.
"in the same time" = > "at the same time"
tomaszslabon
reviewed
Oct 2, 2023
| } | ||
|
|
||
| if isDeposit { | ||
| // If that's the case, the wallet was already done their |
There was a problem hiding this comment.
" the wallet was already done" => "the wallet has already done"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here we introduce some improvements regarding two mechanisms important for wallet actions execution:
Harden the wallet main UTXO lookup mechanism
The goal of that mechanism is to determine the plain-text wallet main UTXO whose hash is registered in the
Bridgecontract. The plain-text version is necessary to construct wallet transactions on Bitcoin.The current version of the main UTXO lookup mechanism looks just at the last five confirmed transactions targeting the wallet public key hash to determine the plain text main UTXO of the wallet. This mechanism is not ideal as it doesn't recognize the difference between true wallet transactions and arbitrary transfers to the wallet public key hash that can be made by anyone. That means it is enough to craft several arbitrary transfers to block the main UTXO lookup and prevent the given wallet from performing actions requested by the wallet coordinator.
To address that problem, we are improving the mechanism to take the full transaction history into account. To make it efficient, we are taking just transaction hashes first and fetching full transaction data only for the latest transactions, where the chance to find the wallet UTXO is the highest.
Harden the wallet sync check mechanism
The goal of this mechanism is to ensure the previous wallet transaction on Bitcoin chain was properly proved to the Bridge contract. This must be ensured before initiating new wallet transactions in order to maintain proper Bitcoin transaction ordering enforced by the Bridge contract. (see #3559 for further reference)
The current version of this mechanism was a naive implementation that checked whether the wallet main UTXO comes from the latest Bitcoin transaction or, if there was no main UTXO, the wallet doesn't have a transaction history. Additionally, this implementation required the mempool to be empty for both cases. This logic is prone to spam transactions sending funds to wallet addresses arbitrarily. Such spam transactions can cause the wallet to abandon all actions proposed by the coordinator.
Here we fix that by using a more sophisticated mechanism:
For wallets having a registered main UTXO, it is enough to check whether their registered UTXO is still among the confirmed unspent outputs from the Bitcoin network standpoint. In order to do that check, we are leveraging ElectrumX
listunspentmethod that returns outputs not used as inputs by any (either confirmed or mempool) transaction. If a wallet uses their main UTXO to produce another transaction,listunspentwill not show it andEnsureWalletSyncedBetweenChainwill detect this state drift preventing to start another action.For fresh wallets which don't have main UTXO yet, the situation is more complicated. In that case, we are additionally taking mempool UTXOs into account. If there are no UTXOs at all, that implies the wallet has not produced any (either confirmed or mempool) Bitcoin transaction so far. If some UTXOs targets the wallet, we need to check whether they are spam or actually result of proper wallet transaction. We do this by checking the first input of each transaction. Very first transactions of wallets are always deposit sweeps and all their inputs must point to revealed deposit. If the first input refers to a deposit in that case, that means the wallet already produced their first transaction on Bitcoin and no other action should be taken until the corresponding SPV proof is submitted to the Bridge. Otherwise, such a transaction is spam. If all transactions are spam, the wallet can safely start the given action.