stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

Feat/native segwit

Open jcnelson opened this issue 2 years ago • 3 comments

This PR adds support for both stacking and mining with native segwit addresses. The feat/pox-2-unlock branch is merged into this branch, so let's get that reviewed and merged before doing this one.

Highlights:

PoX 2

  • The pox-2 contract will now accept a { version: (buff 1), hashbytes: (buff 32) } tuple for a PoX reward address, which may represent any of the following scriptPubKeys:

    • p2pkh
    • p2sh
    • p2wpkh-p2sh
    • p2wsh-p2sh
    • p2wpkh
    • p2wsh
    • p2tr
  • The PoxAddress type, which represents committed PoX payouts in a LeaderBlockCommitOp, can now represent all legacy address types as well as p2wpkh, p2wsh, and p2tr addresses, as well as their Clarity tuple representations for pox-2.

Burnchain Parser

  • The Bitcoin block downloader and transaction parser are epoch-gated so that at Epoch 2.1, transactions that contain native segwit inputs and outputs will be considered.

  • The Bitcoin address parser can now decode p2wpkh, p2sh, and p2tr scriptPubKeys

  • The vendored Bitcoin transaction parsing library has been patched to generate segwit sighashes for the purposes of mining with segwit.

Burn Operations Processing

  • We no longer care about the contents of a Bitcoin transaction's scriptSig or witness. We never did at the consensus level, but now BurnchainSigner no longer represents this information, and all the code that inadvertently depended on this has been updated to not to so.

  • LeaderKeyRegisterOp no longer has an address field. It was never checked by the consensus rules, and it was only used by the node for matching up a LeaderKeyRegisterOp to the keychain's BurnchainSigner (which has been removed).

  • StackStxOp and TransferStxOp do not support segwit, since the recipient address must decode to a Stacks address (and only legacy Bitcoin addresses do this).

Node

  • Using segwit is a configuration option now, which must be turned on explicitly. It will only take effect in Epoch 2.1.

  • If configured to do so, the miner will spend and create segwit p2wpkh UTXOs instead of legacy UTXOs when mining and registering VRF keys. If a miner has UTXOs for both the legacy and p2wpkh addresses derived from the miner's seed, then the legacy ones will be used prior to Epoch 2.1 and the segwit ones will be used at and after Epoch 2.1. This will permit miners to seamlessly transition to using native segwit at the Epoch 2.1 boundary without a node restart.

  • Leader key registration considers the Bitcoin txid, not the BurnchainSigner or LeaderKeyRegisterOp address field when processing tenures.

Vendored Code

The directory stacks-common/src/deps_common/bech32 was vendored from https://github.com/rust-bitcoin/rust-bech32. It was created by the same folks who created our vendored Bitcoin block and transaction parsing library.

jcnelson avatar Sep 08 '22 19:09 jcnelson

Codecov Report

Merging #3283 (0f619fd) into next (7b98978) will decrease coverage by 0.04%. The diff coverage is 34.40%.

@@            Coverage Diff             @@
##             next    #3283      +/-   ##
==========================================
- Coverage   30.82%   30.77%   -0.05%     
==========================================
  Files         277      278       +1     
  Lines      239522   242831    +3309     
==========================================
+ Hits        73832    74737     +905     
- Misses     165690   168094    +2404     
Impacted Files Coverage Δ
clarity/src/vm/docs/mod.rs 0.00% <ø> (ø)
src/burnchains/db.rs 37.60% <0.00%> (-0.53%) :arrow_down:
src/chainstate/burn/db/processing.rs 56.94% <0.00%> (+2.13%) :arrow_up:
src/chainstate/burn/operations/mod.rs 27.33% <ø> (ø)
...rc/chainstate/burn/operations/user_burn_support.rs 0.00% <0.00%> (ø)
src/chainstate/burn/sortition.rs 57.83% <0.00%> (ø)
src/chainstate/coordinator/tests.rs 0.00% <0.00%> (ø)
src/chainstate/stacks/block.rs 30.88% <0.00%> (+0.14%) :arrow_up:
src/chainstate/stacks/boot/contract_tests.rs 0.00% <0.00%> (ø)
src/chainstate/stacks/boot/pox_2_tests.rs 0.00% <0.00%> (ø)
... and 112 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Sep 08 '22 19:09 codecov[bot]

Closes https://github.com/stacks-network/stacks-blockchain/issues/2586

(referencing the issue for anyone following along, it won't let me link the issue from the sidebar)

zone117x avatar Sep 09 '22 14:09 zone117x

Whelp, looks like the grcov package is once again broken. And people wonder why I'm prone to vendoring dependencies.

jcnelson avatar Sep 10 '22 19:09 jcnelson

I will update this PR to pervasively use 32-byte PoX address hashbuff members.

jcnelson avatar Oct 17 '22 15:10 jcnelson

@jcnelson so far segwit addresses in PoX-2 are working great in my e2e tests https://github.com/hirosystems/stacks-blockchain-api/pull/1372

I have some questions around when and why PoXAddressVersion.P2SHP2WPKH // 0x02 and PoXAddressVersion.P2SHP2WSH // 0x03 would ever be used over PoXAddressVersion.P2SH // 0x01.

For example, various PoX related code in stacks.js and the API always use PoXAddressVersion.P2SH // 0x01 when encoding or decoding a bitcoin P2SH / P2SH(P2WSH) / P2SH(P2WPKH) address to and from a PoxAddress (i.e. a { version: (buff 1), hashbytes: (buff 32) }).

This isn't specific to this PR since I think they've been around since PoX-1, but wondering if there's anything special to consider for P2SH(P2WSH) or P2SH(P2WPKH)?

zone117x avatar Oct 24 '22 14:10 zone117x

@zone117x It's for compatibility with Stacks 1.0, which distinguished between the various address types for the purposes of extracting public keys from the scriptSig / witness. There are still a few places that depend on this in the 2.05 consensus rules.

jcnelson avatar Nov 01 '22 19:11 jcnelson

Testing PR https://github.com/stacks-network/stacks-blockchain/pull/3357

igorsyl avatar Nov 01 '22 22:11 igorsyl