Skip to content

Commit 9bc101a

Browse files
mpjunior92claude
andcommitted
fix(types): keep AppSignResponse wire keys backward-compatible
Earlier in this PR I added explicit json: tags on AppSignResponse — "operator_address" / "partial_signature" — alongside the type change to common.Address. That broke wire compat with operators on pre-PR builds: the old serialization used the unexported defaults ("OperatorAddress", "PartialSignature"), so a node from master couldn't decode responses from a node on this branch and vice versa. The security fix is the type itself (common.Address canonicalises the 20-byte form on both sides of the wire and rules out hex-encoding aliasing). The key rename is incidental. Drop the json: tags so the wire keys revert to "OperatorAddress" / "PartialSignature", keeping the type-level Sybil defense without a coordinated rollout. Added Test_AppSignResponse_WireKeysAreStable that pins the wire keys ("OperatorAddress" / "PartialSignature" present, snake_case absent) so a future maintainer can't silently reintroduce the rename. Existing canonicalization test updated to use the stable wire keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent de1ba8a commit 9bc101a

2 files changed

Lines changed: 41 additions & 11 deletions

File tree

pkg/types/appsign_response_test.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@ import (
1212
// regression where AppSignResponse.OperatorAddress was a string. Two payloads
1313
// that differ only in hex casing must decode to the same common.Address so a
1414
// collector can't be tricked into counting a single responder twice.
15+
//
16+
// The wire keys are the unexported default ("OperatorAddress",
17+
// "PartialSignature") — see the AppSignResponse type doc for why we keep them
18+
// rather than switching to snake_case json tags.
1519
func Test_AppSignResponse_OperatorAddressIsCanonical(t *testing.T) {
1620
// Same address, three encodings: lower-case, upper-case (EIP-55-ish), no 0x prefix on inner hex.
1721
encodings := []string{
18-
`{"operator_address":"0xabcdef1234567890abcdef1234567890abcdef12","partial_signature":{"compressed_bytes":null}}`,
19-
`{"operator_address":"0xABCDEF1234567890ABCDEF1234567890ABCDEF12","partial_signature":{"compressed_bytes":null}}`,
20-
`{"operator_address":"0xAbCdEf1234567890aBcDeF1234567890AbCdEf12","partial_signature":{"compressed_bytes":null}}`,
22+
`{"OperatorAddress":"0xabcdef1234567890abcdef1234567890abcdef12","PartialSignature":{"compressed_bytes":null}}`,
23+
`{"OperatorAddress":"0xABCDEF1234567890ABCDEF1234567890ABCDEF12","PartialSignature":{"compressed_bytes":null}}`,
24+
`{"OperatorAddress":"0xAbCdEf1234567890aBcDeF1234567890AbCdEf12","PartialSignature":{"compressed_bytes":null}}`,
2125
}
2226

2327
expected := common.HexToAddress("0xabcdef1234567890abcdef1234567890abcdef12")
@@ -29,6 +33,29 @@ func Test_AppSignResponse_OperatorAddressIsCanonical(t *testing.T) {
2933
}
3034
}
3135

36+
// Test_AppSignResponse_WireKeysAreStable pins the wire format so that the
37+
// type-safety change to common.Address does not accidentally rename the
38+
// JSON keys and break operators running pre-PR builds.
39+
func Test_AppSignResponse_WireKeysAreStable(t *testing.T) {
40+
addr := common.HexToAddress("0x9095535f04796d223A83c0e1346e7C1D9C6EE6f3")
41+
42+
enc, err := json.Marshal(AppSignResponse{OperatorAddress: addr})
43+
require.NoError(t, err)
44+
45+
var wireFields map[string]json.RawMessage
46+
require.NoError(t, json.Unmarshal(enc, &wireFields))
47+
48+
_, hasOperatorAddress := wireFields["OperatorAddress"]
49+
_, hasPartialSignature := wireFields["PartialSignature"]
50+
require.True(t, hasOperatorAddress, "wire key must remain 'OperatorAddress' (no json tag rename)")
51+
require.True(t, hasPartialSignature, "wire key must remain 'PartialSignature' (no json tag rename)")
52+
53+
_, hasSnakeOperator := wireFields["operator_address"]
54+
_, hasSnakePartial := wireFields["partial_signature"]
55+
require.False(t, hasSnakeOperator, "must not regress to snake_case 'operator_address' wire key")
56+
require.False(t, hasSnakePartial, "must not regress to snake_case 'partial_signature' wire key")
57+
}
58+
3259
// Test_AppSignResponse_RoundTrip checks that the OperatorAddress survives a
3360
// marshal/unmarshal cycle and remains comparable with ==.
3461
func Test_AppSignResponse_RoundTrip(t *testing.T) {

pkg/types/types.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,18 @@ type AppSignRequest struct {
117117

118118
// AppSignResponse contains a partial signature from a node.
119119
//
120-
// OperatorAddress is sent and received as a typed common.Address so that
121-
// JSON encode/decode round-trips through the canonical 20-byte form. This
122-
// rules out aliasing attacks where a malicious responder submits two
123-
// partial signatures under different string encodings of the same address
124-
// (lower-case vs EIP-55, with-or-without 0x prefix) and the collector
125-
// counts both toward the ⌈2n/3⌉ threshold.
120+
// OperatorAddress is a typed common.Address so JSON encode/decode round-trips
121+
// through the canonical 20-byte form. This rules out aliasing attacks where a
122+
// malicious responder submits two partial signatures under different string
123+
// encodings of the same address (lower-case vs EIP-55, with-or-without 0x
124+
// prefix) and the collector counts both toward the ⌈2n/3⌉ threshold.
125+
//
126+
// Wire keys ("OperatorAddress", "PartialSignature") are intentionally derived
127+
// from the unexported default rather than `json:` tags so that the type
128+
// change stays wire-compatible with operators still running pre-PR builds.
126129
type AppSignResponse struct {
127-
OperatorAddress common.Address `json:"operator_address"`
128-
PartialSignature G1Point `json:"partial_signature"`
130+
OperatorAddress common.Address
131+
PartialSignature G1Point
129132
}
130133

131134
// SecretsRequestV1 represents a request for application secrets

0 commit comments

Comments
 (0)