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.

Sign up for GitHub

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

Jump to bottom

[CHIA-1719] Add coin announcement restriction #18853

Merged
merged 3 commits into from
Nov 13, 2024
Merged

[CHIA-1719] Add coin announcement restriction #18853

merged 3 commits into from
Nov 13, 2024

Conversation

Quexington
Copy link
Contributor

No description provided.

@Quexington Quexington added clvm Added Required label for PR that categorizes merge commit message as "Added" for changelog labels Nov 12, 2024
@Quexington Quexington marked this pull request as ready for review November 12, 2024 16:43
@Quexington Quexington requested a review from a team as a code owner November 12, 2024 16:43
@Quexington Quexington changed the title Add coin announcement restriction [CHIA-1719] Add coin announcement restriction Nov 12, 2024
matt-o-how
matt-o-how reviewed Nov 13, 2024
View reviewed changes
chia/wallet/puzzles/custody/restriction_puzzles/wrappers/force_assert_coin_announcement.clsp
(include condition_codes.clib)

(defun check_conditions (conditions)
(if (= (f (f conditions)) ASSERT_COIN_ANNOUNCEMENT)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that this is failing if (f (f conditions)) is called when conditions is empty, but might be worth adding a comment explaining this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

matt-o-how
matt-o-how reviewed Nov 13, 2024
View reviewed changes
chia/wallet/puzzles/custody/restriction_puzzles/wrappers/force_assert_coin_announcement.clsp
@@ -0,0 +1,16 @@
(mod
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment to the top of the file explaining what this file does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Quexington Quexington merged commit fea40d7 into quex.multi_sig_chialisp_drivers Nov 13, 2024
358 of 359 checks passed
@Quexington Quexington deleted the quex.force_coin_announcement_restriction branch November 13, 2024 20:24
@github-actions GitHub Actions
Copy link
Contributor

File Coverage Missing Lines
chia/wallet/puzzles/custody/restriction_puzzles/restrictions.py 92.3% lines 42
Total Missing Coverage
34 lines Unknown 97%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matt-o-how matt-o-how matt-o-how left review comments

Assignees
No one assigned
Labels
Added Required label for PR that categorizes merge commit message as "Added" for changelog clvm coverage-diff
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Quexington @matt-o-how