chore: add warning logs for errors not thrown #31009
Conversation
eth/protocols/snap/handler.go
Outdated
| @@ -277,10 +277,12 @@ func ServiceGetAccountRangeQuery(chain *core.BlockChain, req *GetAccountRangePac | |||
| // Retrieve the requested state and bail out if non existent | |||
| tr, err := trie.New(trie.StateTrieID(req.Root), chain.TrieDB()) | |||
| if err != nil { | |||
| log.Warn("Failed to open account trie", "root", req.Root, "err", err) | |||
There was a problem hiding this comment.
I don’t think adding this warning log makes sense. It could potentially open an attack vector where an attacker sends arbitrary account range query requests with unknown state roots, leading to an overwhelming number of warning logs.
There was a problem hiding this comment.
Okay, how about I change the warning log to debug log?
Also, I notice warning logs from rpc calls:
WARN [01-09|09:18:14.746] Served eth_call conn=127.0.0.1:42494 reqid=3 duration="92.539µs" err="missing trie node 5eb6e371a698b8d68f665192350ffcecbbbf322916f4b51bd79bb6887da3f494 (path ) layer stale"
WARN [01-09|09:18:29.763] Served eth_call conn=127.0.0.1:36890 reqid=3 duration="105.81µs" err="missing trie node 5eb6e371a698b8d68f665192350ffcecbbbf322916f4b51bd79bb6887da3f494 (path ) layer stale"
From:
Line 481 in 033de2a
This warning is not frequent, but i think it could also open an attack vector. Does it make sense to change it to debug log as well?
There was a problem hiding this comment.
The assumptions for RPC endpoint is different: it is not assumed to be freely accessible from the internet. So crafting RPC spam calls to cause large numbers of logs is not really something that's considered from security PoV.
There was a problem hiding this comment.
@jwasinger Got it, thank you for explanation.
Found two errors useful for debugging snapsync should be printed out.