Conversation
Co-authored-by: Greg Nazario <greg@gnazar.io>
|
Cursor Agent can help with this pull request. Just |
Co-authored-by: Greg Nazario <greg@gnazar.io>
Co-authored-by: Greg Nazario <greg@gnazar.io>
There was a problem hiding this comment.
Pull request overview
This PR removes BLS12-381 from the repo’s public specification/docs and strips BLS-related test scaffolding/step definitions across Rust/Go/.NET test suites, aligning the specs with the stated goal that BLS is not used for transaction signing.
Changes:
- Removed the BLS12-381 section and references from the cryptography specification and related documentation/indexes.
- Removed BLS12-381 state/logic from Rust cucumber test world + steps.
- Deleted BLS-related pending step definitions from Go and .NET test suites and updated status/feature documentation to no longer advertise BLS.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/rust/src/support/world.rs | Removes BLS key/signature state from the Rust test world and related crypto imports. |
| tests/rust/src/steps/transaction_steps.rs | Drops the BLS “signed” marker path in transaction signing assertions. |
| tests/rust/src/steps/multi_sig_steps.rs | Cleans up comments now that BLS aggregation is no longer referenced. |
| tests/rust/src/steps/cryptography_steps.rs | Removes BLS signing/verification/length assertions from cryptography steps. |
| tests/rust/src/steps/account_steps.rs | Removes the BLS account-address placeholder validation branch. |
| tests/rust/SDK_STATUS.md | Updates Rust SDK status docs to remove BLS feature/flag mentions. |
| tests/go/try_steps.go | Removes BLS pending “try/verify” step definitions. |
| tests/go/the_steps.go | Removes pending BLS PoP/aggregation/authenticator steps. |
| tests/go/simulation_steps.go | Removes pending BLS “rogue key attacks” step. |
| tests/go/setup_steps.go | Removes pending BLS aggregation setup step. |
| tests/go/remaining_steps.go | Removes pending BLS scheme identifier step. |
| tests/go/more_steps.go | Removes pending BLS public key/signature/aggregation steps. |
| tests/go/mnemonic_steps.go | Removes pending BLS derivation step. |
| tests/go/misc_steps.go | Removes BLS-related misc steps/test-vector toggles and headings. |
| tests/go/given_steps.go | Removes pending BLS “known vectors” and PoP steps. |
| tests/go/final_steps.go | Removes pending BLS verification/identifier/aggregation steps. |
| tests/go/client_steps.go | Removes pending BLS client steps. |
| tests/go/action_steps.go | Removes pending BLS parsing step. |
| tests/dotnet/StepDefinitions/MoreCryptoSteps.cs | Removes pending BLS step definitions from .NET step bindings. |
| specifications/README.md | Updates spec index to remove BLS12-381 from the cryptography doc description. |
| specifications/03-cryptography.md | Removes the BLS12-381 section and renumbers subsequent sections (hashing, auth keys, derivation, etc.). |
| feature-matrix.md | Removes the BLS12-381 row from the cross-SDK feature matrix. |
| README.md | Removes the BLS feature file entry from the repo layout overview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Document | Description | | ||
| | -------------------------------------------------- | --------------------------------------------- | | ||
| | [00-conventions.md](00-conventions.md) | Conventions, keywords, and compliance levels | | ||
| | [01-core-types.md](01-core-types.md) | AccountAddress, ChainId, TypeTag, StructTag | | ||
| | [02-bcs-serialization.md](02-bcs-serialization.md) | Binary Canonical Serialization format | | ||
| | [03-cryptography.md](03-cryptography.md) | Ed25519, Secp256k1, Secp256r1, hashing | |
There was a problem hiding this comment.
PR description says BLS is being removed from the specification and related documentation, but there are still multiple repo docs referring to BLS/BLS12381 (e.g., AUDIT_REPORT.md, MOCKED_TESTS_REPORT.md, tests//SDK_STATUS.md, tests//PLAN.md). This leaves the documentation set inconsistent after the spec change. Either update those remaining docs to remove/clarify BLS references, or narrow the PR description/scope to match what’s actually being updated here.
Co-authored-by: Greg Nazario <greg@gnazar.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Priority | Missing | Feature Areas | | ||
| | --------- | ------- | -------------------------------------------------------------------------- | | ||
| | Required | ~6 | Transaction submission (4), Error handling (2) | | ||
| | Preferred | ~15 | Simulation (13), Fee-payer (2) | | ||
| | Optional | ~105 | Codegen (34), Indexer (~24), Multi-agent/Fee-payer/Multi-sig/Keyless (~47) | |
There was a problem hiding this comment.
This updated priority breakdown removes BLS from Optional, but the summary above still reports Optional as "~110/250" and Total as "578/739". Please recompute and update those scenario totals to reflect the BLS spec/scenario removal so the plan stays internally consistent.
Co-authored-by: Greg Nazario <greg@gnazar.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove BLS from the specification and related documentation because it is not used for transaction signing.