Skip to content

Feat/coinbase pay to contract #3164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 41 commits into from
Jun 29, 2022
Merged

Feat/coinbase pay to contract #3164

merged 41 commits into from
Jun 29, 2022

Conversation

jcnelson
Copy link
Member

@jcnelson jcnelson commented Jun 8, 2022

EDIT: this PR now supports building coinbase transactions that pay to either a smart contract or an alternative standard principal. The latter permits the miner to pay to a cold storage address automatically.

This PR implements #3101 by altering the TransactionPayload::Coinbase(..) type variant to include an optional smart contract address alternative recipient address, be it a contract or standard principal address. If given, the encoded transaction payload will have ID 0x05, and will include a serialized smart contract identifier principal. When processed in epoch 2.1, the block reward will be transferred to this smart contract address instead of the miner. In addition, I have updated the node software to accept a pay_to_contract contract contract or standard address under the [miner] heading called block_reward_address, which if given, will cause the node to create pay-to-contract alternative recipient coinbases for blocks mined after epoch 2.1 (the node ignores this field in epoch 2.05 and earlier).

I have added test coverage for:

  • the new transaction encoding
  • the static transaction validation tests (which are now epoch-gated to check for this new variant)
  • the account crediting logic
  • the miner's ability to produce a valid history of blocks that pay to a smart contract OR standard principal
  • the relayer's ability to reject pay-to-contract OR pay-to-alt-principal blocks that are mined before epoch 2.1
  • the node's ability to mine pay-to-contract OR pay-to-alt-principal blocks only once epoch 2.1 goes live

The code includes the relevant feature gating to ensure that a pay-to-contract pay-to-alt-principal block can only be successfully preprocessed in epoch 2.1. This prevents a Stacks 2.1-series node from accepting a pay-to-contract pay-to-alt-principal block at all before epoch 2.1 goes live. But, this differs slightly from how a 2.05-series node will handle such a block -- the 2.05-series node will not even be able to decode it, thereby making any attempt to push such a block to it fail with either an HTTP 400 (if sent via HTTP) or a temporary ban (if sent via p2p). I can make the 2.1-series node just as strict by adding feature gates to the p2p and HTTP endpoints, but I'm not sure it will make an appreciable difference in safety. Also, I'm not sure it's a good idea to make 2.1-series nodes ban each other for this, since if it turns out there's an off-by-one error somewhere in the gating logic that causes nodes to produce these blocks before they're supported, the nodes could accidentally all ban each other, leading to a catastrophic network partition. Open to feedback on this.

jcnelson added 23 commits June 7, 2022 21:39
…ress for a block, not the address that receives the coinbase
…inbase variant. Add the codec logic for it. Add epoch 2.1 gating in the static transaction validation logic so that a block that gets preprocessed before 2.1 with a pay-to-contract coinbase will not be treated as valid (which stops a 2.1-series node from even accepting such a block into the staging DB before the feature is supported).
…tely from the miner address, and update the reward disbursment code to send the block reward to this address if it is given. Expand the unit tests to cover this case.
…tic transaction validation logic so that we never store a pay-to-contract block mined before 2.1. Also, API sync with the new coinbase payload type. In addition, when rejecting a block during pre-processing mask NotFound errors so that the code path returns InvalidStacksBlock.
…t the contract, not the miner, receives the expected block rewards
…ses, and update all uses of the coinbase type variant to include the optional contract ID
… the config file, and the current epoch is 2.1
…-contract blocks after epoch 2.1 starts, if configured to do so
@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #3164 (e8c85f8) into next (00ca64b) will decrease coverage by 0.41%.
The diff coverage is 89.81%.

❗ Current head e8c85f8 differs from pull request most recent head 6376520. Consider uploading reports for the commit 6376520 to get more accurate results

@@            Coverage Diff             @@
##             next    #3164      +/-   ##
==========================================
- Coverage   84.19%   83.78%   -0.42%     
==========================================
  Files         268      268              
  Lines      212573   213534     +961     
==========================================
- Hits       178981   178902      -79     
- Misses      33592    34632    +1040     
Impacted Files Coverage Δ
clarity/src/vm/docs/mod.rs 86.30% <ø> (ø)
src/chainstate/stacks/db/mod.rs 87.26% <ø> (ø)
src/cost_estimates/fee_medians.rs 93.27% <ø> (ø)
src/cost_estimates/fee_scalar.rs 90.78% <ø> (ø)
src/main.rs 0.08% <0.00%> (ø)
src/net/download.rs 32.43% <0.00%> (-8.74%) ⬇️
src/net/rpc.rs 31.50% <0.00%> (-0.21%) ⬇️
testnet/stacks-node/src/tests/epoch_21.rs 90.85% <19.67%> (-5.84%) ⬇️
testnet/stacks-node/src/config.rs 56.02% <20.00%> (-0.25%) ⬇️
src/chainstate/stacks/auth.rs 97.82% <80.00%> (-0.03%) ⬇️
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 875eb49...6376520. Read the comment docs.

… it has a microblock parent stream and crosses an epoch boundary) should be an InvalidStacksBlock error, not a NotFoundError
Copy link
Contributor

@pavitthrap pavitthrap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@jcnelson jcnelson requested a review from kantai June 28, 2022 18:03
@jcnelson jcnelson mentioned this pull request Jun 28, 2022
…dules, grouped by functional requirements tested
Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jcnelson
Copy link
Member Author

Thanks for the approvals! Let's get #3180 merged to this PR, and then I'll merge this all into next.

jcnelson added 2 commits June 28, 2022 15:16
Fix/3159
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 17, 2024
@wileyj wileyj deleted the feat/coinbase-pay-to-contract branch March 11, 2025 21:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants