corona: comments state the contract the code holds

The frame walkers and the boundary checks around them explained themselves by
recounting what a malformed frame did before they existed, in one case with
enough detail to reconstruct it. A comment is read by whoever maintains the
code, and what they need is the rule: a count is worth only the bytes behind
it, membership is checked before shape because shape is what the kernels size
themselves from, a Poly carries its own counts and so is walked like the rest.

No behaviour changes; every line here is a comment.

Co-authored-by: Hanzo Dev <[email protected]>

sign/sign.go

@@ -274,7 +274,7 @@ func (party *Party) SignRound2Preprocess(A structs.Matrix[ring.Poly], b structs.
 	// entries named by T; ranging D folded in every commitment a peer sent,
 	// including one keyed by a party id outside T that no MAC ever checked. A
 	// non-member's matrix — of any shape, since nothing validated it — then
-	// reached the adder, where a mismatch used to take the process down.
+	// reached the adder, where its shape decides what gets allocated.
 	for _, j := range T {
 		D_j, ok := D[j]
 		if !ok {

threshold/codec_dos_test.go

@@ -8,11 +8,10 @@ import (
 	"testing"
 )
 
-// TestAPeerCannotEndTheProcessWithALength. Decoding a signature means reading
-// counts a peer wrote, and the lattice decoder allocates from a count before it
-// discovers there is nothing to fill the space with. A fifty-byte message
-// naming 2^40 coefficients ended the process with "makeslice: len out of
-// range" — a node killed by reading its peer's signature.
+// TestAPeerCannotEndTheProcessWithALength pins what a signature frame is held
+// to: every count in it must be backed by bytes that are present. The lattice
+// decoder sizes its allocations from those counts, so each frame is walked
+// before it is decoded and one whose arithmetic does not close is refused.
 func TestAPeerCannotEndTheProcessWithALength(t *testing.T) {
 	le := func(n uint64) []byte {
 		var b [8]byte
@@ -28,7 +27,7 @@ func TestAPeerCannotEndTheProcessWithALength(t *testing.T) {
 	claim := func(polys, levels, n uint64) []byte {
 		return append(append(le(polys), le(levels)...), le(n)...)
 	}
-	// C is a single Poly, not a vector, so it names only levels and
+	// C is a single Poly, not a vector, so it carries only levels and
 	// coefficients.
 	polyClaim := func(levels, n uint64) []byte {
 		return append(le(levels), le(n)...)
@@ -61,7 +60,7 @@ func TestAPeerCannotEndTheProcessWithALength(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			defer func() {
 				if r := recover(); r != nil {
-					t.Fatalf("%d bytes from a peer ended the process: %v", len(tc.frame), r)
+					t.Fatalf("%d bytes from a peer reached the decoder: %v", len(tc.frame), r)
 				}
 			}()
 			var s Signature
@@ -72,8 +71,8 @@ func TestAPeerCannotEndTheProcessWithALength(t *testing.T) {
 	}
 }
 
-// A group key's A is a matrix and was decoded without ever being walked, so it
-// could name rows and coefficients that were not there.
+// A group key's A is a matrix, so its rows and their contents are held to the
+// same rule.
 func TestAGroupKeysMatrixCannotEndTheProcess(t *testing.T) {
 	le := func(n uint64) []byte {
 		var b [8]byte
@@ -109,7 +108,7 @@ func TestAGroupKeysMatrixCannotEndTheProcess(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			defer func() {
 				if r := recover(); r != nil {
-					t.Fatalf("a group key from a peer ended the process: %v", r)
+					t.Fatalf("a group key from a peer reached the decoder: %v", r)
 				}
 			}()
 			var gk GroupKey

threshold/threshold.go

@@ -269,10 +269,9 @@ func (s *Signer) Round2(sessionID int, message string, prfKey []byte, signers []
 	// The agreed signing set. A round-1 message from a party OUTSIDE it is
 	// refused here, at the boundary, before its matrix touches any kernel: the
 	// MAC loop downstream only authenticates the members of `signers`, so a
-	// non-member's D was never checked and — carrying any shape, or none —
-	// reached the matrix routines, where a malformed one used to take the process
-	// down. Membership is the authentication precondition, so it is checked
-	// first.
+	// non-member's D would otherwise reach the matrix routines carrying any shape,
+	// or none, and shape is what those routines size themselves from. Membership
+	// is the authentication precondition, so it is checked first.
 	inSet := make(map[int]struct{}, len(signers))
 	for _, id := range signers {
 		inSet[id] = struct{}{}

threshold/wire.go

@@ -122,11 +122,9 @@ func (s *Signature) UnmarshalBinary(b []byte) error {
 		return fmt.Errorf("corona/wire: %d trailing bytes after Signature", r.Len())
 	}
 
-	// Every frame is walked before it is decoded, because the decoder allocates
-	// from the counts these carry. C is a single Poly and names its own level
-	// and coefficient counts, so it describes bytes that are not there exactly
-	// as a vector can — a twenty-four byte C naming 2^40 levels does not panic,
-	// it allocates until the process is killed.
+	// Every frame is walked before it is decoded, because the decoder sizes its
+	// allocations from the counts these carry. C is a single Poly and carries
+	// its own level and coefficient counts, so it is walked like the rest.
 	if err := wire.ValidatePolyFrame(cBytes); err != nil {
 		return fmt.Errorf("%w: C: %v", ErrWireFrameRejected, err)
 	}
@@ -228,8 +226,8 @@ func (gk *GroupKey) UnmarshalBinary(b []byte) error {
 		return fmt.Errorf("corona/wire: %d trailing bytes after GroupKey", r.Len())
 	}
 
-	// A is a matrix and carries the same kind of counts as everything else here,
-	// so it is walked before it is decoded rather than after.
+	// A is a matrix and carries the same counts a row deeper, so it is walked
+	// before it is decoded.
 	if err := wire.ValidateMatrixPolyFrame(aBytes); err != nil {
 		return fmt.Errorf("%w: A: %v", ErrWireFrameRejected, err)
 	}

wire/frame_test.go

@@ -40,10 +40,10 @@ func TestFramesTheLibraryWroteAreAccepted(t *testing.T) {
 	}
 }
 
-// TestADeclaredLengthMustBeBackedByBytes. A peer writes the counts in this
-// frame, and the decoder beneath allocates from them before discovering there
-// is nothing to fill the space with: a fifty-byte message naming 2^40
-// coefficients used to end the process with "makeslice: len out of range".
+// TestADeclaredLengthMustBeBackedByBytes pins the rule every count is held to:
+// a count is worth only the bytes behind it. The decoder beneath sizes its
+// allocations from these counts, so one that outruns the frame has to be
+// refused here rather than acted on there.
 func TestADeclaredLengthMustBeBackedByBytes(t *testing.T) {
 	u64 := func(n uint64) []byte {
 		var b [8]byte

wire/wire.go

@@ -52,17 +52,12 @@ var LatticeWireLimits = codec.Limits{
 //	poly   := levels:u64, n:u64, levels*n × coefficient:u64
 //	matrix := rows:u64, rows × vector
 //
-// A peer writes those counts, so a short frame can declare an enormous one —
-// and the decoder beneath this allocates from the declared count before it
-// discovers there is nothing to fill it with. A fifty-byte message naming 2^40
-// coefficients ends the process: sometimes with "makeslice: len out of range",
-// and sometimes by allocating until the kernel kills it. Reading a peer's
-// signature must not be able to do either.
-//
-// So the rule is the one readLenPrefixed already applies to the outer frame:
-// what is declared has to fit in what remains. That cannot refuse an honest
-// frame, since an honest frame's bytes are all present, and it refuses every
-// frame whose arithmetic does not close.
+// A peer writes those counts, and the decoder beneath this sizes its
+// allocations from a count before it reads the bytes meant to fill it. So a
+// count is worth only the bytes behind it, and the rule at every one is the
+// rule readLenPrefixed applies to the outer frame: what is declared has to fit
+// in what remains. That cannot refuse an honest frame, whose bytes are all
+// present, and it refuses every frame whose arithmetic does not close.
 //
 // MaxLatticeUintSliceLen still caps the shape corona itself will accept, above
 // and beyond what the bytes could hold.
@@ -78,8 +73,8 @@ func ValidateVectorPolyFrame(frame []byte) error {
 }
 
 // ValidatePolyFrame is the same walk for a single Poly, which is what a
-// Signature's challenge is. A Poly names its own level and coefficient counts,
-// so it describes bytes that are not there exactly as a vector can.
+// Signature's challenge is. A Poly carries its own level and coefficient
+// counts, so it needs the same walk a vector does.
 func ValidatePolyFrame(frame []byte) error {
 	if err := frameFits(frame); err != nil {
 		return err