flashbots / flashbots/go-utils

Bug: Context getters have panics, silent overwrites, and inconsistent behavior

Open
#66 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
21
Forks
10
PR merge metrics
No merged PRs in 30d

Description

## Summary

The `rpcserver` context helpers have two concrete issues:
1. Two functions panic on missing values
2. Inconsistent error-handling across getters

---

## Bug 1: Panics

```go
// jsonrpc_server.go:432 - PANICS if sizeKey not in context
func GetRequestSize(ctx context.Context) int {
return ctx.Value(sizeKey{}).(int)
}

// jsonrpc_server.go:436 - PANICS if urlKey not in context
func GetURL(ctx context.Context) *url.URL {
return ctx.Value(urlKey{}).(*url.URL)
}
```

Call these outside a handler (e.g., in a test, a goroutine, or refactored code path) and they crash.

---

## Bug 2: Inconsistent Contracts

| Function | Missing Value Behavior |
|----------|----------------------|
| `GetHighPriority` | Returns `false` |
| `GetSigner` | Returns zero address |
| `GetRequest` | Returns `nil` |
| `GetBuilderNetSentAt` | Returns zero time |
| `GetRequestSize` | **Panics** |
| `GetURL` | **Panics** |

Callers can't know which are safe without reading source.

---

## Proposed Fix

Replace `context.Value()` getters with a typed wrapper (like Gin/Echo):

```go
type RPCContext struct {
Request *http.Request // guaranteed non-nil
URL *url.URL // guaranteed non-nil
RequestSize int
Signer common.Address
// ...
}

func (h *Handler) Method(rctx *RPCContext, args Args) (Result, error) {
rctx.Request.Header.Get("X-Foo") // always safe
}
```

No panics, no magic keys, explicit contracts.

---

## Why Now

Hit this adding header logging across services. Every call site needed nil checks or risked panics. The current design makes simple tasks error-prone.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in jsonrpc_server.go around lines 432-436 and inspect the existing context getters and their call sites. Compare the missing-value behavior across GetHighPriority, GetSigner, GetRequest, GetBuilderNetSentAt, GetRequestSize, and GetURL, then determine the scope of introducing the proposed RPCContext wrapper and verify that missing values no longer panic.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend-api-design
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.