feat(sae): register debug RPC chain inspection methods#154
feat(sae): register debug RPC chain inspection methods#154powerslider wants to merge 11 commits intomainfrom
Conversation
Add `SetHead` (no-op) and `ChainDb` to `ethAPIBackend`, enabling direct registration of `ethapi.NewDebugAPI(b)` for the debug namespace. - Activated methods: - `debug_setHead` - `debug_printBlockchaindbCompact` - `debug_chaindbProperty` - `debug_dbGet` - `debug_dbAncient` - `debug_dbAncients` - `debug_getRawTransaction` TODO: (pending `BlockByNumberOrHash` and `GetReceipts`): - `debug_getRawHeader` - `debug_getRawBlock` - `debug_getRawReceipts` resolves #101 Signed-off-by: Tsvetan Dimitrov (tsvetan.dimitrov@avalabs.org)
powerslider
force-pushed
the
powerslider/101-debug-api
branch
from
4917d44 to
f949b37
Compare
powerslider
changed the title
- Add wantErr and wantNonEmpty fields to rpcTest struct. - Extend testRPC to handle error assertions, void RPCs, and non-deterministic values. - Convert all debug tests from direct CallContext to rpcTest structs.
|
|
||
| for _, tc := range tcs { | ||
| t.Run(tc.method, func(t *testing.T) { | ||
| if tc.wantErr != "" { |
There was a problem hiding this comment.
Overall helpful change I think, but I'm going to pull it out into a separate PR (and give you all the credit ;) )
See #157
sae/rpc_test.go
Outdated
| method: "debug_printBlock", | ||
| args: []any{uint64(0)}, | ||
| want: "", | ||
| wantNonEmpty: true, |
There was a problem hiding this comment.
Seems easy enough to verify an exact value by peeking into implementation just a little bit:
// PrintBlock retrieves a block and returns its pretty printed form.
func (api *DebugAPI) PrintBlock(ctx context.Context, number uint64) (string, error) {
block, _ := api.b.BlockByNumber(ctx, rpc.BlockNumber(number))
if block == nil {
return "", fmt.Errorf("block #%d not found", number)
}
return spew.Sdump(block), nil
}So just get the genesis block, and store the string here
There was a problem hiding this comment.
The spew.Sdump output includes Go pointer addresses (e.g. (*types.Block)(0x123456c7890)) that are non-deterministic across allocations. Even if we read the genesis block from the DB and spew.Sdump it locally, the API's PrintBlock calls BlockByNumber internally which deserializes a separate *types.Block instance - different Go object, different pointer addresses, different string.
Because of this I went with a different appoach where I am extending cmp.Diff's capabilities via cmp.Options to strip the non-deterministic parts (which are the pointer addresses). So this way we even gain better accuracy by actually checking for equality. Check a551dd8 and consider maybe moving this into your extraction PR if we get a consensus of the viability of this approach.
sae/rpc_test.go
Outdated
| args []any | ||
| want any // expected value compared via [cmp.Diff] | ||
| wantErr string // if non-empty, assert ErrorContains instead of comparing want | ||
| wantNonEmpty bool // if true, assert only that the result is non-zero (for non-deterministic values) |
There was a problem hiding this comment.
I think if you implement my other comments, you won't even need this test option.
| var ( | ||
| zeroAddr common.Address | ||
|
|
||
| // goPointerAddr matches Go pointer addresses, e.g. "(0x140003c0280)". |
There was a problem hiding this comment.
Maybe say specifically where this is used?
|
Do you want to change the base branch of this PR to Austin's PR (although that PR than needs Arran's branch merged into it) for the proper diff? |
3 participants
Why this should be merged
Check #101
How this works
Add
SetHead(no-op) andChainDbtoethAPIBackend, enabling direct registration ofethapi.NewDebugAPI(b)for the debug namespace.debug_setHeaddebug_printBlockchaindbCompactdebug_chaindbPropertydebug_dbGetdebug_dbAncientdebug_dbAncientsdebug_getRawTransactionTODO: (pending
BlockByNumberOrHashandGetReceipts):debug_getRawHeaderdebug_getRawBlockdebug_getRawReceiptsresolves #101
Signed-off-by: Tsvetan Dimitrov (tsvetan.dimitrov@avalabs.org)