`eth_getBalance` - Missing block parameter silently defaults to `latest`

[manusw7]

Description

When eth_getBalance is called with only the address (block parameter omitted), Nethermind silently defaults to the latest block and returns a balance (Reth does the same); while Geth, Erigon, and Besu all reject with -32602 "missing value for required argument 1".

Per the execution-apis spec, the block parameter is marked as required.

This behavior is currently codified as intentional by the Nethermind unit test Eth_get_balance_default_block in src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs:101.
That test must be deleted in the fix PR.

Steps to Reproduce

{
  "jsonrpc": "2.0",
  "method": "eth_getBalance",
  "params": ["0xcf1dc766fc2c62bef0b67a8de666c8e67acf35f6"],
  "id": 1
}

Expected (Geth / Erigon / Reth)

{
  "jsonrpc": "2.0",
  "id": 1,
  "error": {
    "code": -32602,
    "message": "missing value for required argument 1"
  }
}

Actual (Nethermind v1.38.0-rc)

{ "jsonrpc": "2.0", "result": "0x46f5e69c5c82ad00", "id": 1 }

Fix Location

JSON-RPC argument-count check at the module-dispatch layer in Nethermind.JsonRpc. The current BlockParameter? blockParameter = null default-to-latest behavior on the handler signature (see Nethermind.JsonRpc/Modules/Eth/IEthRpcModule.cs:95 and EthRpcModule.cs:175-191) needs to become a missing-required-argument rejection at parameter binding.

Notes

• Black-box regression guard: test_34 in feat(eth_getBalance): add tests for EIP-1898 scenarios and error handling rpc-tests#6
• Fix affects all eth_* endpoints whose handler signature uses BlockParameter? = null — review impact across the module.

[DarkLord017]

This exists in other endpoints as well (for eg -> eth_getcode) maybe fix all at once

[LukaszRozmej]

Is this worth fixing at all? I like default parameters and I am not sure what removing defaultness will add. IMO it will only detract.

[manusw7]

Is this worth fixing at all? I like default parameters and I am not sure what removing defaultness will add. IMO it will only detract.

This comes from the fact that in execution-apis spec, the block parameter is marked required. Also all other clients (except Reth) reject calls with missing block params.

Given that, I agree with you that defaultness is more a feature then a bug, so we need to decide which is more important:

1. Strictly match spec and align to other clients
2. Keep default parameters and be less strict with spec

cc: @svlachakis

[svlachakis]

As per our guidelines geth is the gold standard at least for spec'd endpoints but I understand @LukaszRozmej's point. @manusw7 start a thread in the channel in order to get more opinions because that's NM behavior across many endpoints.

Maybe I can raise it in next RPC Standards call and suggest a change on the spec.

[MysticRyuujin]

This was brought up in the ETH R&D Discord and I'll just post my same thoughts here:

The problem with diverging from the spec and having a hidden default is that it allows developers to write shitty code that works with one client because they happen to be testing with that one client, that then doesn't work with the other 4+ clients and breaks as soon as they switch RPC providers. It's a similar argument we had with Besu about accepting decimal numbers over "0x" hex strings.

If we specify that the default is "latest" and if omitted that we accept it as "latest", and then enforce compliance via the hive tests with a test case that explicitly omits the block tag, then fine. But it can't be "opt-in" it needs to be implemented by all clients to avoid the footgun that comes with client diversity.

[manusw7]

@MysticRyuujin completely agree with you.
The paths are either:

1. change spec/hive, same default-to-latest behaviour across clients
2. drop defaultness from NM and adhere to spec/other-clients

[MysticRyuujin]

I don't have a strong preference either way, but convincing all 5 clients to default to latest will be more work and take longer imo.

But, if you want to make that proposal in the execution-apis via a PR and take it to ACD and get consensus, I'll support it.