stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 667 forks source link

[sBTC] Add stx_address() public method to PegOutRequestOp #3647

Closed netrome closed 1 year ago

netrome commented 1 year ago

Background

The stacks coordinator must be able to derive an STX address from a peg out request operation. This address can be derived using the recipient, amount and signature fields already present in the op. However, everyone using sBTC must agree on this scheme so it makes sense to implement this logic in the PegOutRequestOp struct to act as a reference for all users of sBTC.

Definition of done

The PegOutRequestOpstruct has a stx_address method returning a StacksAddress, corresponding to the sBTC holders stacks account.

jcnelson commented 1 year ago

However, the operation does not contain the signed message,

That's because it's not necessary. The data that goes into the message to be signed can be extracted from the sBTC smart contract and Bitcoin state.

Downstream applications (such as the Stacks Coordinator) must therefore fetch the corresponding bitcoin transaction to be able to derive this information. It would be much more convenient if the sender information was stored in the op in the stacks node directly.

  1. We don't have space
  2. Writing data to non-segwit Bitcoin data spaces is expensive. An ounce of effort today will save users literally hundreds of dollars down the road.
netrome commented 1 year ago

Thank you for clarifying this @jcnelson. We'll recover the public key in the Coordinator instead then.

netrome commented 1 year ago
  1. Writing data to non-segwit Bitcoin data spaces is expensive. An ounce of effort today will save users literally hundreds of dollars down the road.

To clarify, I'm not proposing changing anything written to Bitcoin. I'm just proposing to update the internal struct stored in the sortition database and burnchain ops table in the stacks nodes. But I'm completely fine with saving space here.

netrome commented 1 year ago

Jude and I sorted out the confusion in discord. "Sender" was a confusing name. Renaming the field to stx_address. We now agreed that this proposal makes sense.

netrome commented 1 year ago

Thinking some more on this, I realize we don't need to actually store the stx_address as we can recover it given the recipient and amount which are already stored in the op.

However, I think it still makes sense to have this logic in the stacks node as a reference. Therefore I will rephrase this ticket to add a public method pub fn stx_address() on the PegOutRequestOp.