-
Notifications
You must be signed in to change notification settings - Fork 247
Wrapping EverCBOR #7533
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
Wrapping EverCBOR #7533
Conversation
e80864b to
24b02d3
Compare
92dc7d0 to
f8c95f0
Compare
f8c95f0 to
fe60567
Compare
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.
Pull request overview
This PR replaces the direct use of EverCBOR with a new CBOR wrapper abstraction (ccf::cbor). The primary purpose is to simplify CBOR parsing operations throughout the codebase by providing a higher-level, type-safe API.
Key Changes:
- Introduces a new CBOR wrapper library (
src/crypto/cbor.handsrc/crypto/cbor.cpp) that encapsulates EverCBOR operations - Refactors UVM endorsement parsing to use the new wrapper, significantly reducing code complexity (from ~400 lines to ~100 lines in the decode functions)
- Updates error handling to use the new
CBORDecodeErrortype consistently
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/crypto/cbor.h | Defines the new CBOR wrapper API with types and parsing functions |
| src/crypto/cbor.cpp | Implements the CBOR wrapper, including parsing and value extraction methods |
| src/node/uvm_endorsements.cpp | Refactors protected header decoding to use the new CBOR wrapper instead of raw EverCBOR calls |
| cmake/crypto.cmake | Adds the new cbor.cpp to the build configuration |
Comments suppressed due to low confidence (1)
src/node/uvm_endorsements.cpp:343
- Missing closing brace for the
namespace ccfblock. The namespace opened at line 9 should be closed after line 168.
}
}
fe60567 to
de8463e
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Amaury Chamayou <amaury@xargs.fr>
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
This's purely reading CBORs, and lacking more exhausting tests, but is a good starting point, maybe?