Skip to content

node: Consolidate borsch library usage#4892

Open
djb15 wants to merge 1 commit into
wormhole-foundation:mainfrom
djb15:node/borsch-consolidation
Open

node: Consolidate borsch library usage#4892
djb15 wants to merge 1 commit into
wormhole-foundation:mainfrom
djb15:node/borsch-consolidation

Conversation

@djb15

@djb15 djb15 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

I spent some time looking through the existing tests and adding a couple of new ones to check whether we're hitting all the edge cases we care about. Not a place we want to be making a mistake, so I'm happy to abandon these changes if the risks outweigh the upside

errStr: "",
},
{
name: "success -- trailing bytes are ignored",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a change in behaviour, right? Previously I believe this returned an error, and that's probably safer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If gagliardetto doesn't do this then I think that's arguably a bug in that library

@johnsaigle johnsaigle Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got this from an LLM FWIW:


_Borsh rejects trailing bytes. The reference implementation (borsh-rs) treats any unconsumed bytes after deserialization as an error.
The high-level entry points enforce this:

  • from_slice / try_from_slice — after T::deserialize, check if !v_mut.is_empty() and return Err(InvalidData, "Not all bytes read") (borsh-rs/borsh/src/de/mod.rs).
  • from_reader / try_from_reader — after deserializing, attempt to read 1 more byte; success (non-EOF) triggers the same "Not all bytes read" error.
    The low-level deserialize / deserialize_reader methods themselves do not check — they only consume what the type needs. So the "no trailing bytes" rule is enforced at the API boundary, not per-field. The written spec (the README pseudocode) doesn't call it out explicitly, but the reference implementation's behavior is the de facto standard, and the error constant ERROR_NOT_ALL_BYTES_READ makes the stance unambiguous: the entire input must be consumed._

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