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

Modular Clarity Update #1008

Merged
merged 22 commits into from
Dec 11, 2024
Merged

Modular Clarity Update #1008

merged 22 commits into from
Dec 11, 2024

Conversation

setzeus
Copy link
Collaborator

@setzeus setzeus commented Dec 2, 2024

Description

This PR introduces the proposed updates to the existing Clarity protocol in order to support protocol contract updates. This design is based off of the discussed items in #990.

Closes: #990

Changes

Explicitly tracks the 'active' version of the contracts for signers, deposits & withdrawals in the sbtc-registry. Additionally, validation is improved so that only the correct role for each contract type can write into the correct sbtc-registry functions.

Testing Information

Tests were updated accordingly & two new tests were added post-update with an example 'deposit-v1' contract.

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@setzeus setzeus marked this pull request as draft December 2, 2024 17:49
Copy link
Contributor

@hstove hstove left a comment

Choose a reason for hiding this comment

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

Mainly have some feedback around the storage type for protocol contracts. But, overall, the approach looks good!

contracts/contracts/sbtc-deposit-v1.clar Outdated Show resolved Hide resolved
contracts/contracts/sbtc-bootstrap-signers.clar Outdated Show resolved Hide resolved
contracts/contracts/sbtc-registry.clar Outdated Show resolved Hide resolved
contracts/contracts/sbtc-registry.clar Outdated Show resolved Hide resolved
contracts/contracts/sbtc-registry.clar Show resolved Hide resolved
contracts/contracts/sbtc-registry.clar Outdated Show resolved Hide resolved
@setzeus setzeus marked this pull request as ready for review December 7, 2024 22:45
@setzeus setzeus requested a review from hstove December 7, 2024 22:45
@setzeus setzeus changed the title Modular Clarity Update (draft - do not merge) Modular Clarity Update Dec 7, 2024
@setzeus setzeus requested a review from hozzjss December 7, 2024 22:54
Copy link
Contributor

@hstove hstove 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/contracts/sbtc-deposit-update-test.clar Outdated Show resolved Hide resolved
contracts/tests/sbtc-registry.test.ts Outdated Show resolved Hide resolved
contracts/tests/sbtc-registry.test.ts Outdated Show resolved Hide resolved
contracts/tests/sbtc-registry.test.ts Outdated Show resolved Hide resolved
contracts/tests/sbtc-registry.test.ts Outdated Show resolved Hide resolved
@aldur aldur added this to the sBTC 0.9, mainnet release milestone Dec 9, 2024
@djordon djordon added the clarity The clarity smart contracts. label Dec 9, 2024
contracts/contracts/sbtc-bootstrap-signers.clar Outdated Show resolved Hide resolved
contracts/contracts/sbtc-token.clar Show resolved Hide resolved
contracts/tests/sbtc-bootstrap-signers.test.ts Outdated Show resolved Hide resolved
contracts/tests/sbtc-bootstrap-signers.test.ts Outdated Show resolved Hide resolved
contracts/tests/sbtc-token.test.ts Outdated Show resolved Hide resolved
contracts/Clarinet.toml Show resolved Hide resolved
Copy link
Contributor

@friedger friedger left a comment

Choose a reason for hiding this comment

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

sbtc token transfer requires a print for the memo

@setzeus setzeus force-pushed the clarity/modular branch 3 times, most recently from 9f35d4b to a958230 Compare December 11, 2024 16:13
@djordon
Copy link
Collaborator

djordon commented Dec 11, 2024

@friedger can you please take another look here.

Copy link
Collaborator

@djordon djordon left a comment

Choose a reason for hiding this comment

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

Looks good

@setzeus
Copy link
Collaborator Author

setzeus commented Dec 11, 2024

sbtc token transfer requires a print for the memo

Was completed in an upstream branch! Dismissing for now.

@setzeus setzeus dismissed friedger’s stale review December 11, 2024 23:47

Addresssed in an upstream merge

@setzeus setzeus merged commit 1acc67b into main Dec 11, 2024
4 checks passed
@djordon djordon deleted the clarity/modular branch December 12, 2024 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarity The clarity smart contracts.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants