refactor(rpc): more granular rpc error codes

[lklimek]

Issue being fixed or feature implemented

Map mempool / broadcast errors to specific JSON-RPC application error codes and add a sentinel for broadcast confirmation timeouts.

What was done?

• Return coretypes.ErrBroadcastConfirmationNotReceived when broadcast confirmation times out.
• Add coretypes.ErrBroadcastConfirmationNotReceived and ErrTooManyRequests.
• Add JSON-RPC error codes for mempool/broadcast errors and map Go errors to those codes via rpcErrorCodeFor and helper predicates.
• Add unit test for error-to-code mappings.

How Has This Been Tested?

• New unit test: TestRPCRequestMakeErrorMapsCodes.
• Ran relevant JSON-RPC tests locally and verified broadcast/mempool error flows return the expected codes.

Breaking Changes

• JSON-RPC now returns specific application error codes (–32000..–32099) for mempool/broadcast errors; clients relying on previous generic codes should update error handling.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

Summary by CodeRabbit

• Bug Fixes
  • Refined error handling in broadcast operations with standardized error reporting for context cancellation. Enhanced JSON-RPC responses with dedicated error codes for broadcast confirmation timeouts, rate limiting, mempool capacity constraints, invalid requests, and transaction validation errors. RPC clients now receive more granular diagnostic information for improved error handling.

[coderabbitai]

Walkthrough

The changes introduce standardized application-specific JSON-RPC error codes (-32000 to -32005) and establish a unified error resolution mechanism. New sentinel errors are added to the coretypes package, error mapping in the JSON-RPC layer is refactored from hardcoded values to dynamic resolution, and context cancellation in mempool operations wraps with a standardized error. Test coverage validates the new error-to-code mapping.

Changes

Cohort / File(s)	Summary
Error Constants & Definitions rpc/coretypes/responses.go	Added two new exported error constants: ErrBroadcastConfirmationNotReceived and ErrTooManyRequests; existing ErrInvalidRequest reformatted without logic changes.
Error Code Mapping System rpc/jsonrpc/types/types.go	Introduced six public error code constants (CodeTxAlreadyExists through CodeBroadcastConfirmationNotReceived, -32000 to -32005); refactored MakeError to use dynamic rpcErrorCodeFor resolver; added helpers (isErrTxTooLarge, isErrMempoolFull, isTooManyRequestsErr, isBroadcastConfirmationNotReceivedErr) for error type detection.
Context Cancellation Error Handling internal/rpc/core/mempool.go	Modified context cancellation paths in BroadcastTx and BroadcastTxCommit to wrap ctx.Err() with ErrBroadcastConfirmationNotReceived sentinel error; success paths unchanged.
Error Mapping Test Coverage rpc/jsonrpc/types/types_test.go	Added TestRPCRequestMakeErrorMapsCodes validating error-to-code mapping across invalid request, tx cache, tx size, mempool, timeout, rate limiting, and broadcast confirmation scenarios; updated imports for context, errors, coretypes, and types.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Error resolution logic: Review the rpcErrorCodeFor function and all helper predicates (isErrTxTooLarge, isErrMempoolFull, isTooManyRequestsErr, isBroadcastConfirmationNotReceivedErr) to ensure error chain inspection is robust and handles wrapped errors correctly.
• MakeError refactoring: Verify that the transition from hardcoded mapping to dynamic resolution preserves existing behavior for *RPCError and nil error cases.
• Error wrapping in mempool: Confirm context cancellation errors are correctly wrapped with the new sentinel error while preserving the original cancellation cause.
• Test coverage: Ensure all new error code paths are exercised and assertions accurately reflect expected mappings.

Poem

🐰 With codes now clear and errors dressed,
In standardized attire, we test!
From context's demise to rate-limit's call,
Each error finds its code—a rabbit's protocol.
No more confusion in the JSON-RPC hall,
Just sentinel errors, wrapped and all! 📋

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'refactor(rpc): more granular rpc error codes' clearly and accurately summarizes the main changes: introducing more granular/specific error codes for RPC handling across multiple files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/rpc-error-codes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

rpc/jsonrpc/types/types.go (1)

155-193: Move broadcast confirmation check before deadline check in rpcErrorCodeFor to prevent shadowing.

The issue is verified and accurate. When fmt.Errorf("%w: %w", coretypes.ErrBroadcastConfirmationNotReceived, ctx.Err()) is called in internal/rpc/core/mempool.go (lines 86, 119) with a deadline context, the resulting error wraps both sentinels. The current switch statement in rpcErrorCodeFor checks errors.Is(err, context.DeadlineExceeded) before isBroadcastConfirmationNotReceivedErr(err), causing the first case to match and return CodeTimeout instead of the intended CodeBroadcastConfirmationNotReceived.

The fix is essential: reorder the cases to check for broadcast confirmation before the generic deadline timeout. This preserves the specific error code for broadcast failures while still handling pure context.DeadlineExceeded errors correctly. The existing test (line 94 of types_test.go) only covers single-wrapped broadcast errors and does not test the actual dual-wrapped scenario from mempool.go, leaving this shadowing bug uncovered.

🧹 Nitpick comments (4)

rpc/coretypes/responses.go (1)

26-28: New RPC sentinel errors are well-aligned with usage

ErrBroadcastConfirmationNotReceived and ErrTooManyRequests give clear, shared sentinels for RPC-layer mapping, and the messages match the matching logic in rpc/jsonrpc/types. Consider adding brief comments indicating which RPC paths produce these to aid discoverability, but behavior itself looks solid.

internal/rpc/core/mempool.go (1)

84-87: ctx cancellation wrapping cleanly exposes broadcast sentinel + cause

Wrapping the ctx.Done() paths with ErrBroadcastConfirmationNotReceived while preserving ctx.Err() as a cause is a good pattern for downstream error introspection and JSON-RPC mapping. The later commit-wait timeout in the loop intentionally remains a separate error shape, which is fine as long as you’re happy with that behavioral distinction.

Also applies to: 117-120

rpc/jsonrpc/types/types_test.go (1)

80-105: Good coverage of new error-code mapping; consider adding mempool-style broadcast case

The table-driven TestRPCRequestMakeErrorMapsCodes nicely asserts the mapping for the main error shapes (invalid request, mempool errors, timeout, rate limiting, broadcast confirmation). To fully cover the real mempool path, you may want an extra case that mimics internal/rpc/core/mempool.go, where the broadcast error wraps both the sentinel and context.DeadlineExceeded:

  testCases := []struct {
     name string
     err  error
     want ErrorCode
  }{
     {"invalid request", coretypes.ErrInvalidRequest, CodeInvalidRequest},
     {"tx already in cache", types.ErrTxInCache, CodeTxAlreadyExists},
     {"tx too large", types.ErrTxTooLarge{Max: 1, Actual: 2}, CodeTxTooLarge},
     {"mempool full", types.ErrMempoolIsFull{}, CodeMempoolIsFull},
     {"deadline exceeded", context.DeadlineExceeded, CodeTimeout},
     {"too many requests sentinel", coretypes.ErrTooManyRequests, CodeTooManyRequests},
     {"too many requests string", errors.New("too_many_requests: limiter triggered"), CodeTooManyRequests},
     {"broadcast confirmation", fmt.Errorf("%w", coretypes.ErrBroadcastConfirmationNotReceived), CodeBroadcastConfirmationNotReceived},
+    {"broadcast confirmation + deadline",
+      fmt.Errorf("%w: %w", coretypes.ErrBroadcastConfirmationNotReceived, context.DeadlineExceeded),
+      CodeBroadcastConfirmationNotReceived},
  }

That will ensure the mapping behaves as expected for the exact error shape produced by the broadcast endpoints.

rpc/jsonrpc/types/types.go (1)

36-57: Application-specific JSON-RPC error codes are well-structured

The new application-specific ErrorCode constants are correctly placed in the reserved -32000..-32099 server range, and errorCodeString entries are consistent and descriptive. This should make client-side handling of mempool/timeouts/ratelimiting significantly clearer.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 354f002 and 5df3696.

📒 Files selected for processing (4)

• internal/rpc/core/mempool.go (2 hunks)
• rpc/coretypes/responses.go (1 hunks)
• rpc/jsonrpc/types/types.go (3 hunks)
• rpc/jsonrpc/types/types_test.go (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

rpc/jsonrpc/types/types_test.go (3)

rpc/jsonrpc/types/types.go (9)

• RPCRequest (62-67)
• ErrorCode (19-19)
• CodeInvalidRequest (31-31)
• CodeTxAlreadyExists (37-37)
• CodeTxTooLarge (38-38)
• CodeMempoolIsFull (39-39)
• CodeTimeout (40-40)
• CodeTooManyRequests (41-41)
• CodeBroadcastConfirmationNotReceived (42-42)

rpc/coretypes/responses.go (3)

• ErrInvalidRequest (26-26)
• ErrTooManyRequests (28-28)
• ErrBroadcastConfirmationNotReceived (27-27)

types/mempool.go (3)

• ErrTxInCache (11-11)
• ErrTxTooLarge (22-25)
• ErrMempoolIsFull (33-38)

internal/rpc/core/mempool.go (1)

rpc/coretypes/responses.go (1)

• ErrBroadcastConfirmationNotReceived (27-27)

rpc/jsonrpc/types/types.go (2)

rpc/coretypes/responses.go (6)

• ErrZeroOrNegativeHeight (21-21)
• ErrZeroOrNegativePerPage (19-19)
• ErrPageOutOfRange (20-20)
• ErrInvalidRequest (26-26)
• ErrTooManyRequests (28-28)
• ErrBroadcastConfirmationNotReceived (27-27)

types/mempool.go (3)

• ErrTxInCache (11-11)
• ErrTxTooLarge (22-25)
• ErrMempoolIsFull (33-38)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

• GitHub Check: Build (amd64, linux)
• GitHub Check: tests (04)
• GitHub Check: tests (03)
• GitHub Check: tests (01)
• GitHub Check: tests (02)
• GitHub Check: golangci-lint
• GitHub Check: e2e-test (dashcore)
• GitHub Check: e2e-test (rotate)

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.