From 1fbe05292f62e1042129d8ff7d6bc04c51f03435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Thu, 23 Sep 2021 20:01:07 +0200 Subject: [PATCH] Fix `SubmitPoolSyncCommitteeSignatures` API endpoint (#9646) * Fix `SubmitPoolSyncCommitteeSignatures` API endpoint * fix test * fix another test Co-authored-by: Raul Jordan Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- .../rpc/apimiddleware/endpoint_factory.go | 3 +- beacon-chain/rpc/apimiddleware/structs.go | 10 +++---- beacon-chain/rpc/eth/beacon/BUILD.bazel | 1 + beacon-chain/rpc/eth/beacon/sync_committee.go | 8 ++--- .../rpc/eth/beacon/sync_committee_test.go | 29 ++++++++++++------- encoding/bytesutil/bytes.go | 11 ------- encoding/bytesutil/bytes_test.go | 22 -------------- 7 files changed, 31 insertions(+), 53 deletions(-) diff --git a/beacon-chain/rpc/apimiddleware/endpoint_factory.go b/beacon-chain/rpc/apimiddleware/endpoint_factory.go index f9cc2bee86..39d68deb08 100644 --- a/beacon-chain/rpc/apimiddleware/endpoint_factory.go +++ b/beacon-chain/rpc/apimiddleware/endpoint_factory.go @@ -122,7 +122,7 @@ func (f *BeaconEndpointFactory) Create(path string) (*gateway.Endpoint, error) { endpoint.RequestQueryParams = []gateway.QueryParam{{Name: "slot"}, {Name: "committee_index"}} endpoint.GetResponse = &attestationsPoolResponseJson{} endpoint.PostRequest = &submitAttestationRequestJson{} - endpoint.Err = &submitAttestationsErrorJson{} + endpoint.Err = &indexedVerificationFailureErrorJson{} endpoint.Hooks = gateway.HookCollection{ OnPreDeserializeRequestBodyIntoContainer: []gateway.Hook{wrapAttestationsArray}, } @@ -137,6 +137,7 @@ func (f *BeaconEndpointFactory) Create(path string) (*gateway.Endpoint, error) { endpoint.GetResponse = &voluntaryExitsPoolResponseJson{} case "/eth/v1/beacon/pool/sync_committees": endpoint.PostRequest = &submitSyncCommitteeSignaturesRequestJson{} + endpoint.Err = &indexedVerificationFailureErrorJson{} endpoint.Hooks = gateway.HookCollection{ OnPreDeserializeRequestBodyIntoContainer: []gateway.Hook{wrapSyncCommitteeSignaturesArray}, } diff --git a/beacon-chain/rpc/apimiddleware/structs.go b/beacon-chain/rpc/apimiddleware/structs.go index 2a63640f3b..fbeae1b5bb 100644 --- a/beacon-chain/rpc/apimiddleware/structs.go +++ b/beacon-chain/rpc/apimiddleware/structs.go @@ -737,14 +737,14 @@ type eventChainReorgJson struct { // Error handling. // --------------- -// submitAttestationsErrorJson is a JSON representation of the error returned when submitting attestations. -type submitAttestationsErrorJson struct { +// indexedVerificationFailureErrorJson is a JSON representation of the error returned when verifying an indexed object. +type indexedVerificationFailureErrorJson struct { gateway.DefaultErrorJson - Failures []*singleAttestationVerificationFailureJson `json:"failures"` + Failures []*singleIndexedVerificationFailureJson `json:"failures"` } -// singleAttestationVerificationFailureJson is a JSON representation of a failure when verifying a single submitted attestation. -type singleAttestationVerificationFailureJson struct { +// singleIndexedVerificationFailureJson is a JSON representation of a an issue when verifying a single indexed object e.g. an item in an array. +type singleIndexedVerificationFailureJson struct { Index int `json:"index"` Message string `json:"message"` } diff --git a/beacon-chain/rpc/eth/beacon/BUILD.bazel b/beacon-chain/rpc/eth/beacon/BUILD.bazel index dcd26a9bb3..31d140a45e 100644 --- a/beacon-chain/rpc/eth/beacon/BUILD.bazel +++ b/beacon-chain/rpc/eth/beacon/BUILD.bazel @@ -107,6 +107,7 @@ go_test( "@com_github_grpc_ecosystem_grpc_gateway_v2//runtime:go_default_library", "@com_github_prysmaticlabs_eth2_types//:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", + "@com_github_wealdtech_go_bytesutil//:go_default_library", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_protobuf//types/known/emptypb:go_default_library", ], diff --git a/beacon-chain/rpc/eth/beacon/sync_committee.go b/beacon-chain/rpc/eth/beacon/sync_committee.go index 3ce05d6153..97e89ac45b 100644 --- a/beacon-chain/rpc/eth/beacon/sync_committee.go +++ b/beacon-chain/rpc/eth/beacon/sync_committee.go @@ -150,11 +150,11 @@ func (bs *Server) SubmitPoolSyncCommitteeSignatures(ctx context.Context, req *et } func validateSyncCommitteeMessage(msg *ethpbv2.SyncCommitteeMessage) error { - if !bytesutil.IsHexOfLen(msg.BeaconBlockRoot, 64) { - return errors.New("invalid block root format") + if len(msg.BeaconBlockRoot) != 32 { + return errors.New("invalid block root length") } - if !bytesutil.IsHexOfLen(msg.Signature, 192) { - return errors.New("invalid signature format") + if len(msg.Signature) != 96 { + return errors.New("invalid signature length") } return nil } diff --git a/beacon-chain/rpc/eth/beacon/sync_committee_test.go b/beacon-chain/rpc/eth/beacon/sync_committee_test.go index c5b8fd6c0a..d60da198c3 100644 --- a/beacon-chain/rpc/eth/beacon/sync_committee_test.go +++ b/beacon-chain/rpc/eth/beacon/sync_committee_test.go @@ -20,6 +20,7 @@ import ( sharedtestutil "github.com/prysmaticlabs/prysm/shared/testutil" "github.com/prysmaticlabs/prysm/shared/testutil/assert" "github.com/prysmaticlabs/prysm/shared/testutil/require" + bytesutil2 "github.com/wealdtech/go-bytesutil" "google.golang.org/grpc" ) @@ -165,13 +166,17 @@ func TestSubmitPoolSyncCommitteeSignatures(t *testing.T) { } t.Run("Ok", func(t *testing.T) { - _, err := s.SubmitPoolSyncCommitteeSignatures(ctx, ðpbv2.SubmitPoolSyncCommitteeSignatures{ + root, err := bytesutil2.FromHexString("0x" + strings.Repeat("0", 64)) + require.NoError(t, err) + sig, err := bytesutil2.FromHexString("0x" + strings.Repeat("0", 192)) + require.NoError(t, err) + _, err = s.SubmitPoolSyncCommitteeSignatures(ctx, ðpbv2.SubmitPoolSyncCommitteeSignatures{ Data: []*ethpbv2.SyncCommitteeMessage{ { Slot: 0, - BeaconBlockRoot: []byte("0x" + strings.Repeat("0", 64)), + BeaconBlockRoot: root, ValidatorIndex: 0, - Signature: []byte("0x" + strings.Repeat("0", 192)), + Signature: sig, }, }, }) @@ -197,19 +202,23 @@ func TestSubmitPoolSyncCommitteeSignatures(t *testing.T) { require.Equal(t, true, ok, "could not retrieve custom error metadata value") assert.DeepEqual( t, - []string{"{\"failures\":[{\"index\":0,\"message\":\"invalid block root format\"}]}"}, + []string{"{\"failures\":[{\"index\":0,\"message\":\"invalid block root length\"}]}"}, v, ) }) } func TestValidateSyncCommitteeMessage(t *testing.T) { + root, err := bytesutil2.FromHexString("0x" + strings.Repeat("0", 64)) + require.NoError(t, err) + sig, err := bytesutil2.FromHexString("0x" + strings.Repeat("0", 192)) + require.NoError(t, err) t.Run("valid", func(t *testing.T) { msg := ðpbv2.SyncCommitteeMessage{ Slot: 0, - BeaconBlockRoot: []byte("0x" + strings.Repeat("0", 64)), + BeaconBlockRoot: root, ValidatorIndex: 0, - Signature: []byte("0x" + strings.Repeat("0", 192)), + Signature: sig, } err := validateSyncCommitteeMessage(msg) assert.NoError(t, err) @@ -219,21 +228,21 @@ func TestValidateSyncCommitteeMessage(t *testing.T) { Slot: 0, BeaconBlockRoot: []byte("invalid"), ValidatorIndex: 0, - Signature: []byte("0x" + strings.Repeat("0", 192)), + Signature: sig, } err := validateSyncCommitteeMessage(msg) require.NotNil(t, err) - assert.ErrorContains(t, "invalid block root format", err) + assert.ErrorContains(t, "invalid block root length", err) }) t.Run("invalid block root", func(t *testing.T) { msg := ðpbv2.SyncCommitteeMessage{ Slot: 0, - BeaconBlockRoot: []byte("0x" + strings.Repeat("0", 64)), + BeaconBlockRoot: root, ValidatorIndex: 0, Signature: []byte("invalid"), } err := validateSyncCommitteeMessage(msg) require.NotNil(t, err) - assert.ErrorContains(t, "invalid signature format", err) + assert.ErrorContains(t, "invalid signature length", err) }) } diff --git a/encoding/bytesutil/bytes.go b/encoding/bytesutil/bytes.go index e91539222b..1ed97e9a98 100644 --- a/encoding/bytesutil/bytes.go +++ b/encoding/bytesutil/bytes.go @@ -366,14 +366,3 @@ func IsHex(b []byte) bool { } return hexRegex.Match(b) } - -// IsHexOfLen checks whether the byte array is a hex number prefixed with '0x' and containing the required number of digits. -func IsHexOfLen(b []byte, length uint64) bool { - if b == nil { - return false - } - matches := hexRegex.Match(b) - // Add 2 to account for '0x' - expectedLen := int(length) + 2 - return matches && len(b) == expectedLen -} diff --git a/encoding/bytesutil/bytes_test.go b/encoding/bytesutil/bytes_test.go index 1cae5a8523..4412c394bd 100644 --- a/encoding/bytesutil/bytes_test.go +++ b/encoding/bytesutil/bytes_test.go @@ -398,28 +398,6 @@ func TestIsHex(t *testing.T) { } } -func TestIsHexOfLen(t *testing.T) { - tests := []struct { - b []byte - l uint64 - result bool - }{ - {nil, 0, false}, - {[]byte(""), 0, false}, - {[]byte("0x"), 2, false}, - {[]byte("0x0"), 2, false}, - {[]byte("foo"), 3, false}, - {[]byte("1234567890abcDEF"), 16, false}, - {[]byte("XYZ4567890abcDEF1234567890abcDEF1234567890abcDEF1234567890abcDEF"), 64, false}, - {[]byte("0x1234567890abcDEF1234567890abcDEF1234567890abcDEF1234567890abcDEF"), 64, true}, - {[]byte("1234567890abcDEF1234567890abcDEF1234567890abcDEF1234567890abcDEF"), 64, false}, - } - for _, tt := range tests { - isHex := bytesutil.IsHexOfLen(tt.b, tt.l) - assert.Equal(t, tt.result, isHex) - } -} - func TestSafeCopyRootAtIndex(t *testing.T) { tests := []struct { name string