Skip to content

Commit

Permalink
RPC omit errors when empty (#124)
Browse files Browse the repository at this point in the history
* Omit errors in JSON RPC if empty and omit response in case of an error

* Address deprecation warning for the linter config
  • Loading branch information
Stefan-Ethernal authored Dec 3, 2024
1 parent 1f69650 commit 10406a1
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 23 deletions.
14 changes: 7 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
---
run:
timeout: 5m
skip-dirs:
- state/runtime/fakevm
- state/runtime/instrumentation
- test
- ci
- mocks


linters:
enable:
- whitespace # Tool for detection of leading and trailing whitespace
Expand Down Expand Up @@ -50,6 +44,12 @@ issues:
linters:
- gosec
- lll
exclude-dirs:
- state/runtime/fakevm
- state/runtime/instrumentation
- test
- ci
- mocks
include:
- EXC0012 # EXC0012 revive: Annoying issue about not having a comment. The rare codebase has such comments
- EXC0014 # EXC0014 revive: Annoying issue about not having a comment. The rare codebase has such comments
11 changes: 6 additions & 5 deletions rpc/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (h *Handler) registerService(service Service) {
fv: mv.Func,
}
var err error
if fd.inNum, fd.reqt, err = validateFunc(funcName, fd.fv, true); err != nil {
if fd.inNum, fd.reqt, err = validateFunc(funcName, fd.fv); err != nil {
panic(fmt.Sprintf("jsonrpc: %s", err))
}
// check if last item is a pointer
Expand All @@ -217,10 +217,11 @@ func (h *Handler) registerService(service Service) {
}

func (h *Handler) getFnHandler(req Request) (*serviceData, *funcData, Error) {
const endpointNameParts = 2
methodNotFoundErrorMessage := fmt.Sprintf("the method %s does not exist/is not available", req.Method)

callName := strings.SplitN(req.Method, "_", 2) //nolint:mnd
if len(callName) != 2 { //nolint:mnd
callName := strings.SplitN(req.Method, "_", endpointNameParts)
if len(callName) != endpointNameParts {
return nil, nil, NewRPCError(NotFoundErrorCode, methodNotFoundErrorMessage)
}

Expand All @@ -238,9 +239,9 @@ func (h *Handler) getFnHandler(req Request) (*serviceData, *funcData, Error) {
return service, fd, nil
}

func validateFunc(funcName string, fv reflect.Value, isMethod bool) (inNum int, reqt []reflect.Type, err error) {
func validateFunc(funcName string, fv reflect.Value) (inNum int, reqt []reflect.Type, err error) {
if funcName == "" {
err = fmt.Errorf("getBlockNumByArg cannot be empty")
err = fmt.Errorf("function name cannot be empty")
return
}

Expand Down
4 changes: 2 additions & 2 deletions rpc/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ type Request struct {
type Response struct {
JSONRPC string `json:"jsonrpc"`
ID interface{} `json:"id"`
Result json.RawMessage `json:"result"`
Error *ErrorObject `json:"error"`
Result json.RawMessage `json:"result,omitempty"`
Error *ErrorObject `json:"error,omitempty"`
}

// NewResponse returns Success/Error response object
Expand Down
8 changes: 4 additions & 4 deletions services/datacom/datacom.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,23 @@ func (d *Endpoints) signSequence(signedSequence types.SignedSequenceInterface) (
// Verify that the request comes from the sequencer
sender, err := signedSequence.Signer()
if err != nil {
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode, "failed to verify sender")
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "failed to verify sender")
}

if sender != d.sequencerTracker.GetAddr() {
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode, "unauthorized")
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "unauthorized")
}

// Store off-chain data by hash (hash(L2Data): L2Data)
if err = d.db.StoreOffChainData(context.Background(), signedSequence.OffChainData()); err != nil {
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode,
return nil, rpc.NewRPCError(rpc.DefaultErrorCode,
fmt.Errorf("failed to store offchain data. Error: %w", err).Error())
}

// Sign
signature, err := signedSequence.Sign(d.privateKey)
if err != nil {
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Errorf("failed to sign. Error: %w", err).Error())
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, fmt.Errorf("failed to sign. Error: %w", err).Error())
}
// Return signature
return signature, nil
Expand Down
4 changes: 4 additions & 0 deletions services/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ func (s *Endpoints) GetStatus() (interface{}, rpc.Error) {
rowCount, err := s.db.CountOffchainData(ctx)
if err != nil {
log.Errorf("failed to get the key count from the offchain_data table: %v", err)

return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "failed to retrieve data from the storage")
}

lastSynchronizedBlock, err := s.db.GetLastProcessedBlock(ctx, string(synchronizer.L1SyncTask))
if err != nil {
log.Errorf("failed to get last block processed by the synchronizer: %v", err)

return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "failed to retrieve data from the storage")
}

return types.DACStatus{
Expand Down
4 changes: 3 additions & 1 deletion services/status/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ func TestEndpoints_GetStatus(t *testing.T) {
name: "failed to count offchain data",
countOffchainDataErr: errors.New("test error"),
getLastProcessedBlock: 2,
expectedError: errors.New("failed to retrieve data from the storage"),
},
{
name: "failed to count offchain data and last processed block",
countOffchainDataErr: errors.New("test error"),
getLastProcessedBlockErr: errors.New("test error"),
expectedError: errors.New("failed to retrieve data from the storage"),
},
}

Expand All @@ -50,7 +52,7 @@ func TestEndpoints_GetStatus(t *testing.T) {
Return(tt.countOffchainData, tt.countOffchainDataErr)

dbMock.On("GetLastProcessedBlock", mock.Anything, mock.Anything).
Return(tt.getLastProcessedBlock, tt.getLastProcessedBlockErr)
Return(tt.getLastProcessedBlock, tt.getLastProcessedBlockErr).Maybe()

statusEndpoints := NewEndpoints(dbMock)

Expand Down
8 changes: 4 additions & 4 deletions services/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (z *Endpoints) GetOffChainData(hash types.ArgHash) (interface{}, rpc.Error)
data, err := z.db.GetOffChainData(context.Background(), hash.Hash())
if err != nil {
log.Errorf("failed to get the offchain requested data from the DB: %v", err)
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode, "failed to get the requested data")
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "failed to get the requested data")
}

return types.ArgBytes(data.Value), nil
Expand All @@ -45,7 +45,7 @@ func (z *Endpoints) GetOffChainData(hash types.ArgHash) (interface{}, rpc.Error)
func (z *Endpoints) ListOffChainData(hashes []types.ArgHash) (interface{}, rpc.Error) {
if len(hashes) > maxListHashes {
log.Errorf("too many hashes requested in ListOffChainData: %d", len(hashes))
return "0x0", rpc.NewRPCError(rpc.InvalidRequestErrorCode, "too many hashes requested")
return nil, rpc.NewRPCError(rpc.InvalidRequestErrorCode, "too many hashes requested")
}

keys := make([]common.Hash, len(hashes))
Expand All @@ -56,10 +56,10 @@ func (z *Endpoints) ListOffChainData(hashes []types.ArgHash) (interface{}, rpc.E
list, err := z.db.ListOffChainData(context.Background(), keys)
if err != nil {
log.Errorf("failed to list the requested data from the DB: %v", err)
return "0x0", rpc.NewRPCError(rpc.DefaultErrorCode, "failed to list the requested data")
return nil, rpc.NewRPCError(rpc.DefaultErrorCode, "failed to list the requested data")
}

listMap := make(map[common.Hash]types.ArgBytes)
listMap := make(map[common.Hash]types.ArgBytes, len(list))
for _, data := range list {
listMap[data.Key] = data.Value
}
Expand Down

0 comments on commit 10406a1

Please sign in to comment.