Refactor .py signatures infra#7904
Open
maxtropets wants to merge 10 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Python-side ledger signature handling by introducing a dedicated ccf.signatures module and updating tests/utilities to use a shared “is this a signature transaction?” predicate and shared parsing/verification helpers. It keeps external compatibility by re-exporting legacy names from ccf.ledger.
Changes:
- Added
python/src/ccf/signatures.pyto centralize signature table constants, parsing, and primitive verification helpers. - Refactored
python/src/ccf/ledger.py’sLedgerValidatorsignature validation to use the new shared helpers, while re-exporting prior public symbols for backwards compatibility. - Updated multiple tests and Python utilities to detect signature transactions via
is_signature_transaction()rather than ad-hoc table checks/string matching.
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/recovery.py | Uses ccf.signatures.is_signature_transaction() for signature tx detection in recovery corruption/selection helpers. |
| tests/reconfiguration.py | Switches signature parsing to parse_classical_signatures() and uses structured payload fields. |
| tests/governance_history.py | Refactors signature schema/position checks to use shared signature-tx detection + structured parsing. |
| tests/e2e_operations.py | Consolidates signature transaction detection via ccf.signatures.is_signature_transaction(). |
| python/src/ccf/split_ledger.py | Uses shared signature-tx detection when splitting ledgers around signature transactions. |
| python/src/ccf/signatures.py | New module defining signature table registry, parsers, and primitive verifiers. |
| python/src/ccf/ledger.py | Re-exports signature APIs for compatibility and refactors validator signature verification flow. |
| python/src/ccf/ledger_viz.py | Uses shared signature-tx detection for labeling signature entries in visualization. |
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
achamayou
reviewed
May 21, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ts/CCF into f/refactor-py-sig-infra
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motto: consolidate signature verification in one place, and benefit from it greatly when introducing PQC COSE signatures in the future.
I still plan to keep a split for verifying classical/cose, but for cose we'll extend the table itself in the future and will iterate over multiple cose signatures instead, and will try to get rid of classical signatures somewhere in the future.