From 80c577e0d45aad251c5c325c895b245f8dfae27e Mon Sep 17 00:00:00 2001 From: Bruce Riley Date: Fri, 19 Jan 2024 10:01:19 -0600 Subject: [PATCH] Code review rework --- node/pkg/query/request.go | 25 ++++++++++++++++++++++--- node/pkg/query/response.go | 4 ++++ node/pkg/watchers/solana/ccq.go | 27 --------------------------- whitepapers/0013_ccq.md | 4 ++-- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/node/pkg/query/request.go b/node/pkg/query/request.go index 53806f84a0..0f9ed2d657 100644 --- a/node/pkg/query/request.go +++ b/node/pkg/query/request.go @@ -13,6 +13,8 @@ import ( ethCommon "github.com/ethereum/go-ethereum/common" ethCrypto "github.com/ethereum/go-ethereum/crypto" + + solana "github.com/gagliardetto/solana-go" ) // MSG_VERSION is the current version of the CCQ message protocol. @@ -138,7 +140,16 @@ type SolanaAccountQueryRequest struct { Accounts [][SolanaPublicKeyLength]byte } -const SolanaPublicKeyLength = 32 +// Solana public keys are fixed length. +const SolanaPublicKeyLength = solana.PublicKeyLength + +// According to the Solana spec, the longest comment string is nine characters. Allow a few more, just in case. +// https://pkg.go.dev/github.com/gagliardetto/solana-go/rpc#CommitmentType +const SolanaMaxCommitmentLength = 12 + +// According to the spec, the query only supports up to 100 accounts. +// https://github.com/solana-labs/solana/blob/9d132441fdc6282a8be4bff0bc77d6a2fefe8b59/rpc-client-api/src/request.rs#L204 +const SolanaMaxAccountsPerQuery = 100 func (saq *SolanaAccountQueryRequest) AccountList() [][SolanaPublicKeyLength]byte { return saq.Accounts @@ -246,6 +257,10 @@ func (queryRequest *QueryRequest) UnmarshalFromReader(reader *bytes.Reader) erro return fmt.Errorf("excess bytes in unmarshal") } + if err := queryRequest.Validate(); err != nil { + return fmt.Errorf("unmarshaled request failed validation: %w", err) + } + return nil } @@ -947,6 +962,10 @@ func (saq *SolanaAccountQueryRequest) UnmarshalFromReader(reader *bytes.Reader) return fmt.Errorf("failed to read commitment len: %w", err) } + if len > SolanaMaxCommitmentLength { + return fmt.Errorf("commitment string is too long, may not be more than %d characters", SolanaMaxCommitmentLength) + } + commitment := make([]byte, len) if n, err := reader.Read(commitment[:]); err != nil || n != int(len) { return fmt.Errorf("failed to read commitment [%d]: %w", n, err) @@ -997,8 +1016,8 @@ func (saq *SolanaAccountQueryRequest) Validate() error { if len(saq.Accounts) <= 0 { return fmt.Errorf("does not contain any account entries") } - if len(saq.Accounts) > math.MaxUint8 { - return fmt.Errorf("too many account entries") + if len(saq.Accounts) > SolanaMaxAccountsPerQuery { + return fmt.Errorf("too many account entries, may not be more that %d", SolanaMaxAccountsPerQuery) } for _, acct := range saq.Accounts { // The account is fixed length, so don't need to check for nil. diff --git a/node/pkg/query/response.go b/node/pkg/query/response.go index 38c54c2cbc..99baaa124d 100644 --- a/node/pkg/query/response.go +++ b/node/pkg/query/response.go @@ -255,6 +255,10 @@ func (msg *QueryResponsePublication) Unmarshal(data []byte) error { return fmt.Errorf("excess bytes in unmarshal") } + if err := msg.Validate(); err != nil { + return fmt.Errorf("unmarshaled response failed validation: %w", err) + } + return nil } diff --git a/node/pkg/watchers/solana/ccq.go b/node/pkg/watchers/solana/ccq.go index 3dacb641e8..7e6b4d4fbd 100644 --- a/node/pkg/watchers/solana/ccq.go +++ b/node/pkg/watchers/solana/ccq.go @@ -174,30 +174,3 @@ func (w *SolanaWatcher) ccqHandleSolanaAccountQueryRequest(ctx context.Context, w.ccqSendQueryResponse(queryRequest, query.QuerySuccess, resp) } - -/* -func (s *SolanaWatcher) testQuery(ctx context.Context, logger *zap.Logger) { - rCtx, cancel := context.WithTimeout(ctx, rpcTimeout) - defer cancel() - acc, err := solana.PublicKeyFromBase58("Jito4APyf642JPZPx3hGc6WWJ8zPKtRbRs4P815Awbb") - if err != nil { - logger.Error("SOLTEST: failed to parse account", zap.Error(err)) - return - } - - slot := uint64(239676280) - logger.Info("SOLTEST: doing read", zap.Any("commitment", s.commitment), zap.Any("account", acc), zap.Uint64("slot", slot)) - info, err := s.rpcClient.GetAccountInfoWithOpts(rCtx, acc, &rpc.GetAccountInfoOpts{ - Encoding: solana.EncodingBase64, - Commitment: s.commitment, - MinContextSlot: &slot, - }) - - if err != nil { - logger.Error("SOLTEST: read failed", zap.Error(err)) - return - } - - logger.Info("SOLTEST: read succeeded", zap.Uint64("slot", info.Context.Slot), zap.Any("data", info.Value.Data)) -} -*/ diff --git a/whitepapers/0013_ccq.md b/whitepapers/0013_ccq.md index a37f535b8f..e44b52bd93 100644 --- a/whitepapers/0013_ccq.md +++ b/whitepapers/0013_ccq.md @@ -391,7 +391,7 @@ uint32 response_len #### Solana Query Responses -1. sol_account (query type 10) Response Body +1. sol_account (query type 4) Response Body ```go u64 slot_number @@ -404,7 +404,7 @@ uint32 response_len - The `slot_number` is the slot number returned by the query. - The `block_time_us` is the timestamp of the block associated with the slot. - The `block_hash` is the block hash associated with the slot. - - The array of results returns the data for each account queried. + - The array of results array returns the data for each account queried. ```go u64 lamports