fix: attach correct-axis, nil-free shares to ErrByzantineData#411
fix: attach correct-axis, nil-free shares to ErrByzantineData#411evan-forbes wants to merge 1 commit into
Conversation
Two related fixes in extendeddatacrossword.go for the Byzantine evidence returned by Repair: - Wrong-axis / nil-hole evidence: the orthogonal-completion loops in solveCrosswordRow/solveCrosswordCol attached the rebuilt OTHER-axis buffer to an ErrByzantineData that names the orthogonal axis. They now attach a copy of the axis the error actually names, with the rebuilt share inserted (no nil hole). The same-axis returns now attach a snapshot taken before rebuildShares decodes in place, so pre-decode nils are preserved. - verifyEncoding mutating the caller slice: it wrote the rebuilt share into the caller-owned row/column slice and deferred a reset to nil, leaving a nil hole in the shares the caller attaches to the error. It now operates on a defensive copy and never mutates its input. Adds regression tests covering the orthogonal wrong-axis branch (shares belong to the named axis, contain the corrupt share, and have no nil at the rebuilt position) and the verifyEncoding non-mutation invariant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where ErrByzantineData could be populated with incorrect shares or 'nil holes' due to in-place mutations during the crossword solving process. Key changes include taking snapshots of shares before rebuilding, introducing a sharesWithRebuiltShare helper to correctly associate shares with the named axis, and modifying verifyEncoding to work on a copy of the data to avoid mutating the caller's slice. Comprehensive regression tests have been added to ensure that Byzantine evidence is accurate and that input slices remain unmutated. I have no feedback to provide as there were no review comments.
|
converting to a draft while I take a deeper look. I not actually sure we want to patch this |
1 participant
Fixes two related issues in the
ErrByzantineData.Sharesevidence returned byRepair:solveCrosswordRow/solveCrosswordColattached the rebuilt other-axis buffer to an error that names the orthogonal axis; they now attach a copy of the named axis with the rebuilt share inserted (and the same-axis returns snapshot committed shares before in-place decoding to preserve nils).verifyEncodingmutating the caller slice: it wrote the rebuilt share into the caller-owned row/column slice and deferred a reset to nil, leaving a nil hole in the attached shares; it now uses a defensive copy and never mutates its input.These
Repair/BEFP code paths are not reachable so this is strictly hardening. Bad-encoding fraud proofs and reconstruction are no longer enabled and not planned to be maintained. This is fixed simply because it is cheap, correct hardening.🤖 Generated with Claude Code