-
Notifications
You must be signed in to change notification settings - Fork 663
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
PoX-4 signer key read/write with base Rust tests #4209
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## next #4209 +/- ##
==========================================
+ Coverage 82.91% 82.96% +0.04%
==========================================
Files 429 429
Lines 302304 302748 +444
==========================================
+ Hits 250650 251169 +519
+ Misses 51654 51579 -75 ☔ View full report in Codecov by Sentry. |
Looks like formatting failed in CI, have to run the following before committing:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minimal comments/thoughts, really glad to see the work here!
Will go a long way to making PoX-related Rust tests easier / increasing code coverage for the rest of PoX-4 tasks.
Strange, I have auto-formatting on. Must be a setting mismatch somewhere. I'll tend to the changes as soon as possible. Appreciate the quick review. |
Make sure you use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @MarvinJanssen!
The direction of this draft looks good. While I haven't pulled this locally yet, I wanted to initiate a discussion on potentially creating common abstractions to further streamline and condense the tests. I've left a detailed comment regarding this. Looking forward to your thoughts.
Yep I saw that, I had old settings in there. Thank you for the pointer! |
f2c3a35
to
bf7f841
Compare
Overall looks good but please address my comments first. Thanks! |
@wileyj this one is good to go I believe. |
@wileyj I had asked to avoid merging this before reviewing |
I don't think Jesse was in that discussion. |
ahh my mistake. was looking over the PR comments/approvals and it appeared good to go. |
Description
This PR adds pox4 signer key read/write and useful helpers to interact with pox4 functions. The focus is to add base Rust tests to be used as a foundation for upcoming work items like #4059, #4058, #4147, and so on. There is also a basic check to make sure that a signer key is 33 bytes and to prevent key reuse.
Applicable issues
Additional info (benefits, drawbacks, caveats)
I kept this PR as narrow as possible for the purpose of unblocking other work and to (hopefully) make a merge straightforward. Processing the signer keys, calculating the aggregate key, and the signer StackerDB work is slated for subsequent PRs.
Comprehensive PoX4 smart contract tests will also be introduced in a different PR. (Pending
clarunit
tooling.)Checklist
docs/rpc/openapi.yaml
andrpc-endpoints.md
for v2 endpoints,event-dispatcher.md
for new events)clarity-benchmarking
repobitcoin-tests.yml