Skip to content

sync: borsh feature is silently broken - Response/ValueResponse/RawDecidedValue Borsh impls are unsatisfiable for every Context (missing borsh impls for ExtendedCommitCertificate) #1612

Open

Description

@miha1llumi

Bug report: malachitebft-sync borsh feature is silently broken (unsatisfiable Borsh bounds)

Component: malachitebft-sync (crates/sync), malachitebft-core-types (crates/core-types)
Versions observed: tag v0.8.0 (as pinned by circlefin/arc-node) and workspace v0.8.2
Severity: build/API correctness bug (not a security vulnerability)

Summary

The borsh feature of the sync crate compiles successfully but its Response /
ValueResponse / RawDecidedValue Borsh (de)serialization impls are unsatisfiable for every
possible Context
, because core-types never implements BorshSerialize/BorshDeserialize
for ExtendedCommitCertificate (or ExtendedCommitSignature). Any downstream consumer that
enables the feature and actually uses the decode surface gets a compile error
(E0277); a consumer that only compiles gets a silently dead decode surface.

Reproduction

# consumer Cargo.toml
malachitebft-sync = { version = "0.8", features = ["borsh"] }
// any use of the advertised decode surface fails for every Ctx:
let resp = malachitebft_sync::rpc::Response::<MyCtx>::deserialize_reader(&mut buf)?;
// error[E0277]: the trait bound `ExtendedCommitCertificate<MyCtx>: BorshDeserialize`
//               is not satisfied
//               required for `RawDecidedValue<MyCtx>` to implement `BorshDeserialize`

Note the trap: cargo check -p malachitebft-sync --features borsh passes — the feature
looks healthy until the impls are actually used.

Root cause

crates/sync/src/ser/borsh.rs:

  • impl BorshDeserialize for Response<Ctx> (lines 69–77) requires
    ValueResponse<Ctx>: BorshDeserialize;
  • impl BorshDeserialize for ValueResponse<Ctx> (lines 91–104) requires
    RawDecidedValue<Ctx>: BorshDeserialize;
  • impl BorshSerialize/BorshDeserialize for RawDecidedValue<Ctx> (lines 106–115 / 117–128)
    require ExtendedCommitCertificate<Ctx>: BorshSerialize (line 108) /
    ExtendedCommitCertificate<Ctx>: BorshDeserialize (line 119) and call
    ExtendedCommitCertificate::deserialize_reader (line 123).

But in core-types, there are no Borsh impls for ExtendedCommitCertificate or
ExtendedCommitSignature anywhere in the crate (grep across the whole workspace: zero
matches). Manual Borsh impls exist for CommitCertificate, PolkaCertificate and
RoundCertificate — the extended variants were evidently missed.

The impls still compile because an unsatisfiable where-clause on an impl definition is not
an error — it only makes the impls unusable. Nothing in the workspace enables
sync's borsh feature (it is not in default features and has no in-tree consumer), so no
CI job ever exercises it and the breakage went unnoticed.

Impact

  • Downstream consumers (archival/indexing services, cross-process persistence of sync
    responses) that enable the documented borsh feature get a build-time breakage that is
    impossible to satisfy from their side (ExtendedCommitCertificate's borsh impls are
    internal to core-types).
  • Worse for diagnostics: feature-only compilation succeeds, so the defect surfaces far away
    from its cause, as an opaque E0277 in consumer code.

Suggested fix

  1. Add BorshSerialize/BorshDeserialize impls for ExtendedCommitCertificate and
    ExtendedCommitSignature in core-types/src/ser/borsh.rs, mirroring the existing manual
    impls for CommitCertificate (decide and document the canonical encoding for the
    extension payload carried by ExtendedCommitSignature).
  2. Add a CI job exercising the feature combination, e.g.
    cargo check -p malachitebft-sync --features borsh plus a small encode/decode roundtrip
    test, so future feature-bitrot is caught.

Environment

  • Rust workspace edition 2021, MSRV 1.88, borsh 1.x (de_strict_order, derive).
  • Verified by grep over the full workspace tree and by a compile-fail probe
    (E0277: the trait bound ExtendedCommitCertificate<...>: BorshDeserialize is not satisfied ... required for RawDecidedValue<...> to implement BorshDeserialize).

Activity

  1. romac commented on Nov 9, 2026

    @romac
    Contributor

    Thanks for the report, we are looking into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions