Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #835 +/- ##
==========================================
+ Coverage 70.37% 70.39% +0.02%
==========================================
Files 132 132
Lines 24737 24836 +99
==========================================
+ Hits 17408 17484 +76
- Misses 7329 7352 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive block validation for company chains, addressing issue #749. The changes introduce a validation-first pattern using CompanyValidateActionData with signature verification via verify_and_get_signer() for all company operations. The PR also adds handling for anonymous users attempting company operations and updates numerous tests to use consistent test keys and proper chain setup.
Changes:
- Implemented
verify_and_get_signer()method for company blocks to verify signatures before validation - Replaced direct authorization checks with
CompanyValidateActionDatavalidation pattern across all company operations - Added anonymous identity checks to prevent anon users from performing company operations
- Updated test infrastructure to use consistent test keys and proper blockchain setup with identity proofs
- Added
--tag latestto npm publish workflow for proper release tagging
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/bcr-ebill-transport/src/handler/mod.rs | Added test utilities for additional test node ID and private key |
| crates/bcr-ebill-transport/src/handler/company_chain_event_processor.rs | Integrated signature verification and validation for incoming company blocks; updated tests to use proper chain setup |
| crates/bcr-ebill-core/src/protocol/blockchain/company/block.rs | Implemented verify_and_get_signer() for extracting and verifying block signers with identity proof validation |
| crates/bcr-ebill-api/src/tests/mod.rs | Added additional test helper functions for consistent test key usage |
| crates/bcr-ebill-api/src/service/company_service.rs | Replaced authorization checks with CompanyValidateActionData validation; added anon identity guards |
| crates/bcr-ebill-api/src/service/bill_service/test_utils.rs | Updated test utilities to use proper chain methods instead of individual block fetching |
| crates/bcr-ebill-api/src/service/bill_service/blocks.rs | Added validation helpers for identity and company blocks; integrated validation for bill signing operations |
| .github/workflows/wasm_release.yml | Added --tag latest to npm publish command for proper package tagging |
75dfe63 to
c17d2a6
Compare
There was a problem hiding this comment.
I see CompanyValidateActionData is only used as a temporary container of information that are used to .validate() the action, yet all the elements are owned.
Maybe an improvement could be to reference those information
pub struct CompanyValidateActionData<'a> {
pub blockchain: &<'a> CompanyBlockchain,
...
}
Yeah, I tried to do this with the |
📝 Description
latesttag to npm release, since otherwise it cries when we do hotfixesRelates to #749
✅ Checklist
Please ensure the following tasks are completed before requesting a review:
cargo fmt.cargo clippy.🚀 Changes Made
See above.
💡 How to Test
Please provide clear instructions on how reviewers can test your changes:
🤝 Related Issues
List any related issues, pull requests, or discussions:
📋 Review Guidelines
Please focus on the following while reviewing: