Skip to content

fix: correct ledger buffer size calculation for Sign function #4376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

fix: correct ledger buffer size calculation for Sign function #4376

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6db17d2
fix: correct ledger buffer size calculation for Sign function
felipemadero Oct 1, 2025
fb4cd8d
docs: clarify ledgerPathSize constant and serialization format
felipemadero Oct 1, 2025
438c4a1
Merge branch 'master' into improve-ledger-buffer-calculation-new
felipemadero Oct 3, 2025
673497d
Merge branch 'master' into improve-ledger-buffer-calculation-new
yacovm Oct 3, 2025
479fccd
Merge branch 'master' into improve-ledger-buffer-calculation-new
yacovm Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions utils/crypto/ledger/ledger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,19 @@ import (
)

const (
rootPath = "m/44'/9000'/0'" // BIP44: m / purpose' / coin_type' / account'
rootPath = "m/44'/9000'/0'" // BIP44: m / purpose' / coin_type' / account'
// ledgerBufferLimit corresponds to FLASH_BUFFER_SIZE for Nano S.
// Modern devices (Nano X, Nano S2, Stax, Flex) support up to 16384 bytes,
// but we use the conservative limit for universal compatibility.
//
// Ref: https://github.com/ava-labs/ledger-avalanche/blob/main/app/src/common/tx.c
ledgerBufferLimit = 8192
ledgerPathSize = 9
// ledgerPathSize is the serialized size of a path suffix (e.g., "0/123") as produced
// by SerializePathSuffix in the ledger library. Format: 1 byte (component count) +
// 4 bytes (first component) + 4 bytes (second component) = 9 bytes total.
//
// Ref: https://github.com/ava-labs/ledger-avalanche-go/blob/main/common.go (SerializePathSuffix)
ledgerPathSize = 9
)

var _ keychain.Ledger = (*Ledger)(nil)
Expand Down Expand Up @@ -103,15 +113,15 @@ func (l *Ledger) SignHash(hash []byte, addressIndices []uint32) ([][]byte, error
}

Copy link
Contributor

@yacovm yacovm Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a general comment, unrelated to this PR, but - shouldn't this code be in https://github.com/ava-labs/ledger-avalanche-go ?

func (l *Ledger) Sign(txBytes []byte, addressIndices []uint32) ([][]byte, error) {
// will pass to the ledger addressIndices both as signing paths and change paths
numSigningPaths := len(addressIndices)
numChangePaths := len(addressIndices)
if len(txBytes)+(numSigningPaths+numChangePaths)*ledgerPathSize > ledgerBufferLimit {
// There is a limit on the tx length that can be parsed by the ledger
// app. When the tx that is being signed is too large, we sign with hash
// instead.
//
// Ref: https://github.com/ava-labs/avalanche-wallet-sdk/blob/9a71f05e424e06b94eaccf21fd32d7983ed1b040/src/Wallet/Ledger/provider/ZondaxProvider.ts#L68
// We pass addressIndices both as signing paths and change paths.
// 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)
Copy link
Contributor

@yacovm yacovm Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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?

bufferSize := 1 + len(addressIndices)*ledgerPathSize + len(txBytes)
if bufferSize > ledgerBufferLimit {
// When the tx that is being signed is too large for the ledger buffer,
// we sign with hash instead.
unsignedHash := hashing.ComputeHash256(txBytes)
return l.SignHash(unsignedHash, addressIndices)
}
Expand Down