fix: correct ledger buffer size calculation for Sign function #4376
Conversation
The previous buffer size calculation incorrectly assumed signing and change paths were stored separately in the ledger buffer. However, the ledger library deduplicates these paths, and the buffer format includes a 1-byte path count prefix. This fixes the calculation to accurately reflect the actual buffer usage.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the ledger buffer size calculation for the Sign function. The previous implementation incorrectly calculated buffer usage by assuming signing and change paths were stored separately, when in fact the ledger library deduplicates these paths.
- Fixed buffer size calculation to account for path deduplication and proper buffer format
- Added comprehensive documentation explaining ledger buffer limits and device compatibility
- Updated comments to reference the correct ledger library implementation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Adds documentation explaining that ledgerPathSize (9 bytes) matches the serialized format produced by SerializePathSuffix in the ledger library: 1 byte for component count + 4 bytes per component for a 2-component path suffix like "0/123".
| } | ||
| return responses, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a general comment, unrelated to this PR, but - shouldn't this code be in https://github.com/ava-labs/ledger-avalanche-go ?
| // The ledger library deduplicates them, so the buffer contains len(addressIndices) paths. | ||
| // Buffer format: 1 byte (path count) + paths + transaction bytes | ||
| // | ||
| // Ref: https://github.com/ava-labs/ledger-avalanche-go/blob/main/common.go (ConcatMessageAndChangePath) |
There was a problem hiding this comment.
The typescript file represents the previous implementation so why is the typescript file not changed accordingly?
Also how does the golang file represent the new logic?
|
moving this to golang SDK ava-labs/avalanche-tooling-sdk-go#182 |
2 participants
Why this should be merged
The previous buffer size calculation incorrectly assumed signing and change paths were stored separately in the ledger buffer. However, the ledger library deduplicates these paths, and the buffer format includes a 1-byte path count prefix. This fixes the calculation to accurately reflect the actual buffer usage.
How this works
How this was tested
Need to be documented in RELEASES.md?