Skip to content
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

Use is-in-mainnet in pox-4.clar #4117

Conversation

friedger
Copy link
Collaborator

@friedger friedger commented Dec 3, 2023

Description

This PR changes the way how pox-4 contracts for mainnet and testnet are created. Instead of string concatenation, we now use is-in-mainnet in clarity.

Applicable issues

Additional info (benefits, drawbacks, caveats)

pox_4_body is only used to create pox-4 contract for mainnet and testnet. It is not used for creating pox-4 contract with other constants.

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

LGTM!

[contracts.pox-4]
path = "./contracts/pox/pox-4-testnet.clar"
path = "../../stackslib/src/chainstate/stacks/boot/pox-4.clar"
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Copy link
Collaborator

@hugocaillard hugocaillard left a comment

Choose a reason for hiding this comment

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

LGTM! thx @friedger

@friedger friedger merged commit 7b47c47 into feat/pox-4-disallow-stacking-during-prepare-phase Dec 4, 2023
1 check passed
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

I wish I'd had a chance to review this before it got merged, because this patch could use some improvement. Namely, the organization of the ADDRESS_VERSION constants are confusing. If you look in pox-4.clar, you'll see code comments to the effect that the four address versions here are included in a separate file, and yet here they are. Could you send another PR that cleans up pox-4.clar's ADDRESS_VERSIONs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants