Skip to content

Switch from postcard to cbor for the Bitcoin app#264

Merged
bigspider merged 1 commit into
masterfrom
cbor
May 19, 2026
Merged

Switch from postcard to cbor for the Bitcoin app#264
bigspider merged 1 commit into
masterfrom
cbor

Conversation

@bigspider
Copy link
Copy Markdown
Contributor

This makes it easier to keep the wire protocol forward-compatible.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR switches the Bitcoin app/client wire protocol from postcard/serde serialization to CBOR via minicbor, aiming to make message evolution more forward-compatible through stable numeric indices.

Changes:

  • Replaces serde derives with minicbor Encode/Decode derives across common message and error types.
  • Updates app and client request/response serialization paths from postcard to minicbor.
  • Adjusts affected response enum shapes and handler/test pattern matches.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/bitcoin/common/src/message/mod.rs Defines CBOR encoding for Bitcoin wire protocol message types.
apps/bitcoin/common/src/errors.rs Adds stable CBOR indices for app error variants.
apps/bitcoin/common/Cargo.toml Replaces runtime serde dependency with minicbor.
apps/bitcoin/client/src/client.rs Switches client request serialization and response parsing to minicbor.
apps/bitcoin/client/Cargo.toml Adds minicbor and removes direct postcard dependency.
apps/bitcoin/app/src/main.rs Switches app request parsing and response serialization to minicbor.
apps/bitcoin/app/src/handlers/sign_psbt.rs Updates PSBT signing response construction/tests for new response shape.
apps/bitcoin/app/src/handlers/get_master_fingerprint.rs Updates master fingerprint response construction/tests for new response shape.
apps/bitcoin/app/Cargo.toml Adds minicbor and removes postcard dependency.
Comments suppressed due to low confidence (1)

apps/bitcoin/common/src/message/mod.rs:104

  • This field-bearing enum variant is not marked #[cbor(map)], so it is encoded positionally instead of with stable field keys. That leaves AccountCoordinates::WalletPolicy outside the documented forward-compatible encoding scheme and makes adding optional fields to this variant a breaking wire change.
    #[n(0)]
    WalletPolicy(#[n(0)] WalletPolicyCoordinates),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/bitcoin/app/src/main.rs Outdated
Comment thread apps/bitcoin/client/src/client.rs Outdated
Comment thread apps/bitcoin/common/src/message/mod.rs
@bigspider bigspider marked this pull request as ready for review May 19, 2026 09:02
@bigspider bigspider requested a review from a team as a code owner May 19, 2026 09:02
This makes it easier to keep the wire protocol forward-compatible.
@bigspider bigspider merged commit 83fd989 into master May 19, 2026
76 checks passed
@bigspider bigspider deleted the cbor branch May 19, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants