test(Ledger_suite): DEFI-2949: Replace cddl crate in the block-schema test#10799
Merged
Conversation
The `block_encoding_agrees_with_the_schema` test validated the CBOR encoding of ICRC-1/ICRC-2 ledger blocks against `block.cddl` using the `cddl` crate, which pulled a large dependency tree into the repo that is used nowhere else. Port `block.cddl` onto the in-repo `generic_value_predicate` combinators (the same ones `icrc3::schema` / `icrc107::schema` use) in a new `ic_icrc1::block_schema` module, and drop the `cddl` dependency along with two stale Bazel `cddl` deps and the now-dead `block.cddl` data deps. Closed-map (unknown-key) strictness is preserved via a local `only_keys` combinator; the `block.cddl` file is kept as the documented spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the external cddl dependency previously used to validate ICRC-1/ICRC-2 ledger block CBOR encodings in tests, replacing it with an in-repo schema validator implemented using generic_value_predicate combinators (mirroring existing ICRC-3/ICRC-107 validators). It also cleans up related Bazel/Cargo deps and lockfile entries while retaining block.cddl as the documented spec.
Changes:
- Replace
cddl-based CBOR validation in the state-machine proptest withic_icrc1::block_schema::validate. - Add a new
ic_icrc1::block_schemamodule implementing a strict (closed-map) port ofblock.cddlplus unit tests. - Remove
cddlfrom Cargo/Bazel deps and updateCargo.lockaccordingly.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rs/ledger_suite/tests/sm-tests/src/lib.rs | Switch schema validation from cddl to ic_icrc1::block_schema::validate. |
| rs/ledger_suite/tests/sm-tests/Cargo.toml | Drop the cddl dependency. |
| rs/ledger_suite/tests/sm-tests/BUILD.bazel | Remove cddl dep and block.cddl data dependency. |
| rs/ledger_suite/test_utils/in_memory_ledger/BUILD.bazel | Remove unused block.cddl data dependency. |
| rs/ledger_suite/icrc1/src/lib.rs | Export the new block_schema module. |
| rs/ledger_suite/icrc1/src/block_schema.rs | Add the new schema validator and its unit tests. |
| rs/ledger_suite/icrc1/ledger/Cargo.toml | Drop the cddl dev-dependency. |
| rs/ledger_suite/icrc1/ledger/BUILD.bazel | Remove cddl dep and block.cddl test data dep. |
| rs/ledger_suite/icrc1/BUILD.bazel | Remove stale Bazel dependency on cddl. |
| Cargo.lock | Remove cddl and related transitive crates (incl. old clap tree), normalize remaining entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eplace-cddl-crate # Conflicts: # Cargo.lock
… CBOR `validate` decoded the block bytes and then called `encoded_block_to_generic_block`, which panics (`.expect`) when CBOR decodes but cannot be converted into a block (e.g. non-text map keys or unsupported CBOR types). A validator returning `Result` should not panic on malformed input. Decode the CBOR once, keep the self-describe tag check, and convert the already-decoded value via a new fallible `try_generic_block_from_value` helper. Add a regression test that feeds unconvertible-but-decodable CBOR and expects `Err` instead of a panic. Addresses a GitHub Copilot review comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The initial port placed the `block.cddl` validator in the production `ic-icrc1` library, even though only tests use it. Move it into the `sm-tests` (testonly) crate instead, so no production code changes. The validator now checks the self-describe tag directly on the encoded bytes (the three CBOR bytes D9 D9 F7) and reuses the existing public `encoded_block_to_generic_block`, so the fallible decode helper added to `ic-icrc1::blocks` is reverted. The malformed-block rejection checks are kept as a `block_encoding_schema_catches_malformed_blocks` test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With the `cddl` crate gone, nothing validates `block.cddl` anymore, so it was unenforced documentation that could silently drift. The CDDL grammar is already reproduced inline in the `block_schema` module next to the predicates that enforce it, and the original file remains in git history. Update the doc comments that referenced the file accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mbjorkqvist
changed the title
cddl crate in the block-schema test…mments The block schema is now validated by the in-repo `block_schema` predicates rather than the `cddl` crate, so refer to it as the ledger block CBOR schema instead of the "CDDL spec"/"CDDL schema validator". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
basvandijk
approved these changes
Jul 17, 2026
basvandijk
left a comment
basvandijk
left a comment
Collaborator
There was a problem hiding this comment.
Cargo.Bazel.json.lock+87 -1,711
Nice. Thanks!
Contributor
|
Force merged because it's only related to tests and reviewers are unavailable until next week |
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.
The
block_encoding_agrees_with_the_schematest validated the CBOR encoding of ICRC-1/ICRC-2 ledger blocks againstblock.cddlusing thecddlcrate.cddlis used in exactly this one place, yet it pulls ~12 transitive crates into the repo that are used nowhere else (plus WASM tooling and a duplicate oldclap 3.2.25).This PR removes the
cddldependency by re-implementing the block schema check with the in-repogeneric_value_predicatecombinators — the same approach the siblingicrc3::schemaandicrc107::schemavalidators already use — in a test-onlyblock_schemamodule in thesm-testscrate. No production code changes.only_keyscombinator, kept local because the ICRC-3/107 validators intentionally stay open for forward-compatibility.cddldependency everywhere: Cargo manifests,Cargo.lock, thecrate.specinbazel/rust.MODULE.bazel, the Bazel crate-index pins, and the BUILD deps (including two stale ones).block.cddlfile; its CDDL grammar is reproduced inline in the validator, and the file remains in git history.