search

stacks-network / sbtc

Repo containing sbtc
GNU General Public License v3.0
286 stars 9 forks source link

[Feature]: Safeguard withdrawal functionality #960

Open Jiloc opened 19 hours ago

Jiloc commented 19 hours ago

1. Description

Withdrawals aren't supported, but we have enough live code that a withdrawal request will disrupt signer operations. We should remove the code that allows for this to happen.

1.1 Context & Purpose

When constructing a bitcoin transaction, we currently get all pending and accepted deposits and withdrawal requests for inclusion in the bitcoin transaction. Unfortunately, this behavior will fail validation for the signers, so we should change it.

2. Technical Details:

The "core" code that needs to be removed is: https://github.com/stacks-network/sbtc/blob/dae208e258c0df4e8d2aa817473f144047700777/signer/src/transaction_coordinator.rs#L1171-L1175

Once that code is removed -- as well the follow-on code -- we should be good, since the coordinator will not include withdrawal requests.

2.1 Acceptance Criteria:

3. Related Issues and Pull Requests (optional):

aldur commented 18 hours ago

PR that introduced tech-debt: https://github.com/stacks-network/sbtc/pull/849

Did we add code for withdrawals in this PR, specifically?

Jiloc commented 17 hours ago

Not really, it’s the PR that added validation, so I probably shouldn’t have marked it specifically. I believe the (partial) withdrawal support itself was introduced across multiple PRs alongside the deposits functionality. For example, when we added support in the block_event to capture deposits prints events from the smart contracts, we also did the same for withdrawals. If you think it’s worth investigating further, I can compile a list of related PRs