[Feature]: Deposits transactions are fetched from the mempool rather than a finalized block

[technovision99]

(High) - Deposits transactions are fetched from the mempool rather than a finalized block

1. Description

When deposit requests are validated, they are fetched from the bitcoin client using the getrawtransaction rpc call: https://github.com/stacks-network/sbtc/blob/ffbe79e0238825f3546e9429bbf89042703823df/sbtc/src/deposits.rs#L125-L138 However, as the comment indicates, this rpc call will fetch transactions that are either in a block or still pending in the mempool. If the deposit transaction is not included in a block it cannot be considered canonical on the bitcoin chain and this may result in sBTC being minted without a corresponding deposit.

This can be remedied by counting the number of block confirmations for a given fetched block.

[djordon]

On the signers side, this is mitigated by the signers only fetching pending deposits that are included in the "canonical" blockchain.

https://github.com/stacks-network/sbtc/blob/c6610791561c4d93c309f58ef703283848d12ae4/signer/src/storage/postgres.rs#L326-L374

[djordon]

https://github.com/stacks-network/sbtc/blob/ffbe79e0238825f3546e9429bbf89042703823df/sbtc/src/deposits.rs#L128-L131 Whoops, that comment is silly.

[djordon]

This was closed by https://github.com/stacks-network/sbtc/pull/679.

[evonide]

@djordon Question regarding the number of confirmations sBTC enforces on Bitcoin blocks.

On the signers side, this is mitigated by the signers only fetching pending deposits that are included in the "canonical" blockchain.

The SQL statement above walks recursively back from the chain tip and ensures that we don't walk further than the provided context_window. However, it doesn't seem to enforce a minimum.

https://github.com/stacks-network/sbtc/pull/679 says

Have the load_latest_deposit_requests function fetch and store all validated deposits that have been confirmed.

This doesn't specify how many confirmations we would like though.

Both fixes (also considering https://github.com/stacks-network/sbtc/pull/675) seem to correctly avoid the scenario where we fetch any transaction from the mempool but this still leaves open if the processed blocks can be trusted. How do we ensure there is a sufficient number of confirmations for the blocks we process?

[djordon]

Both fixes (also considering #675) seem to correctly avoid the scenario where we fetch any transaction from the mempool but this still leaves open if the processed blocks can be trusted. How do we ensure there is a sufficient number of confirmations for the blocks we process?

Well, we don't have a minimum number of blocks. All we require is for the transaction to be confirmed and then we act on it, trying to sweep in the funds and mint sBTC to the recipient once the sweep has been confirmed.

If there is a reorg on bitcoin that affects the deposit, it will necessarily affect the sweep transaction as well (since it uses the deposit and was confirmed afterwards), and both will then re-enter the bitcoin mempool. The signers should eventually notice the reorg and try to sweep it in if it gets confirmed again.

On the stacks side the transaction minting sBTC to the recipient will be affected as well. Post the Stacks Nakamoto hard fork, bitcoin reorgs imply Stacks reorgs. This means the contract call transaction will be reorged as well (assuming it's been confirmed). If a miner tries to pick up and execute the transaction it should fail once https://github.com/stacks-network/sbtc/pull/493 lands, since we'll have a "don't confirm if there was a reorg check" in the smart contract.

[evonide]

That makes sense thanks for the great clarification :)!