Skip to content

Fix multi-backend signature decoding during restore - #438

Open
tinnendo wants to merge 1 commit into
hyperledger-labs:mainfrom
perun-network:fix/multi-backend-restore-decode
Open

tinnendo wants to merge 1 commit into
hyperledger-labs:mainfrom
perun-network:fix/multi-backend-restore-decode

Conversation

@tinnendo

@tinnendo tinnendo commented Mar 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR fixes channel restore failures in multi-backend use cases.

Previously, persisted channel data could decode signatures via the global wallet.DecodeSig(...) path, which tries all registered backends sequentially on the same reader. In multi-backend setups, that can consume signature bytes with the wrong backend decoder before the correct backend gets a chance, causing restore failures.

This change makes restore-time signature decoding backend-aware wherever participant backend information is already available.

Changes

  • add backend-aware signature decoding when a participant backend is known
  • use participant backend IDs during transaction decoding in restore paths
  • keep the existing global fallback when backend context is unavailable
  • add focused tests for:
    • backend-specific SigDec
    • sparse signature decoding with participant backends
    • fallback behavior for participants with multiple backend IDs
    • TransactionDec with participant backend context
    • keyvalue restorer guards for unexpected keys and trailing bytes

Why

The core issue is that the generic multi-backend decode path is unsafe on a shared reader:
the first backend decoder may consume bytes even when it ultimately fails.

This PR avoids that ambiguity in restore and persistence paths by decoding with the correct backend whenever the participant metadata already identifies it.

Result

  • restore works again in multi-backend scenarios such as sim + eth
  • single-backend behavior remains unchanged
  • restore-time decoding becomes deterministic and better tested

Validation

  • go test

@tinnendo
tinnendo force-pushed the fix/multi-backend-restore-decode branch 2 times, most recently from 947bd2a to 0aabb73 Compare March 30, 2026 18:06
Signed-off-by: Hendrik Amler <hendrik@perun.network>
@tinnendo
tinnendo force-pushed the fix/multi-backend-restore-decode branch from 0aabb73 to bd33a90 Compare March 30, 2026 18:08
@tinnendo
tinnendo requested a review from Copilot April 8, 2026 12:00

Copilot AI left a comment

Copy link
Copy Markdown

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 fixes restore-time signature decoding failures in multi-backend setups by making signature decoding backend-aware whenever participant backend information is available, avoiding unsafe “try all backends on the same reader” behavior during restore.

Changes:

  • Add backend-specific signature decoding support (SigDec backend selection, DecodeSparseSigsForParts, and TransactionDec).
  • Update keyvalue restore path to decode transactions/signatures with participant backend context and add stricter restorer guards.
  • Add focused unit tests covering backend-aware decoding and restore-time decoding invariants (unexpected keys, trailing bytes).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
wallet/sig.go Adds backend-aware decoding in SigDec and introduces DecodeSparseSigsForParts with fallback behavior.
wallet/backend.go Adds decodeSigForBackend helper to decode signatures using a specific backend.
wallet/address.go Adds SingleBackendID helper to extract a single backend ID from a participant map.
channel/transaction.go Introduces TransactionDec to decode transactions using participant backend IDs for signature decoding.
channel/persistence/keyvalue/restorer.go Uses participant backend IDs when decoding persisted transactions/signatures; adds key-suffix guard and trailing-bytes detection.
wallet/sig_test.go Adds tests for backend-specific SigDec and sparse signature decoding with participant backend context/fallback.
channel/transaction_dec_test.go Adds test ensuring TransactionDec decodes correctly using participant backend IDs.
channel/persistence/keyvalue/persistrestorer_internal_test.go Adds tests ensuring restore rejects unexpected keys and trailing bytes in persisted values.

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

Comment on lines +254 to +255
if actual := i.its[0].Key(); !strings.HasSuffix(actual, key) {
i.err = errors.Errorf("unexpected iterator key %q, expected suffix %q", actual, key)

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

readNextValue uses strings.HasSuffix(actual, key) to validate the iterator key. This can incorrectly accept unexpected keys that merely end with the expected key (e.g., ...:mycurrent would pass for key="current"), weakening the new guard.

Consider additionally checking that the byte immediately preceding the suffix is the channel DB separator (":"), or otherwise validating the exact key format produced by channelDB(...)+":"+key.

Suggested change
if actual := i.its[0].Key(); !strings.HasSuffix(actual, key) {
i.err = errors.Errorf("unexpected iterator key %q, expected suffix %q", actual, key)
expectedSuffix := ":" + key
if actual := i.its[0].Key(); !strings.HasSuffix(actual, expectedSuffix) {
i.err = errors.Errorf("unexpected iterator key %q, expected suffix %q", actual, expectedSuffix)

Copilot uses AI. Check for mistakes.
Comment thread wallet/sig.go
Comment on lines 49 to 54
type SigDec struct {
Sig *Sig
BackendID int
Sig *Sig
// BackendID optionally selects the backend-specific signature decoder. If it
// is nil, decoding falls back to the global multi-backend decoder.
BackendID *BackendID
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

SigDec is an exported type, and changing BackendID from an int/value field to a *BackendID is a breaking API change for downstream users constructing wallet.SigDec{...}.

If preserving backwards compatibility matters, consider keeping the old field (deprecated) or adding a new field (e.g., BackendIDPtr *BackendID) / constructor helper, and translating internally.

Copilot uses AI. Check for mistakes.
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