ChainSafe / forest

🌲 Rust Filecoin Node Implementation
https://forest.chainsafe.io
Apache License 2.0
632 stars 153 forks source link

Error while deserializing `EthFilterSpec` #4826

Open elmattic opened 4 days ago

elmattic commented 4 days ago

Describe the bug

We don't support deserializing of "topics" correctly.

To reproduce

Run Forest and this Curl command:

curl -X POST 'http://127.0.0.1:2345/rpc/v1' \
    -H 'Content-Type: application/json' \
    --data '{"jsonrpc":"2.0","id":1,"method":"Filecoin.EthGetLogs","params":[{"blockHash": "0xa158a536fe66e18b8cbf5c392384d840483f84732e2a2ed8d9fb68cedcffef54", "address": ["0x51e1f72b655528de2d4d88e70bd53774db8d0b0c"], "topics": ["0x90890809c654f11d6e72a28fa60149770a0d11ec6c92319d6ceb2bb0a4ea1a15"]}]}'

Log output

You should see:

{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"error deserializing parameter","data":{"error":"invalid type: string \"0x90890809c654f11d6e72a28fa60149770a0d11ec6c92319d6ceb2bb0a4ea1a15\", expected a sequence","index":0,"name":"eth_filter","type":"forest_filecoin::lotus_json::LotusJson<forest_filecoin::rpc::methods::eth::types::EthFilterSpec>"}}}

Expected behavior

We should be able to deserialize this type.

We should have complete test coverage.

Screenshots

Environment (please complete the following information):

Other information and links

Note I have already fixed topics here to allow for null topics.

Spec: https://github.com/filecoin-project/lotus/blob/master/chain/types/ethtypes/eth_types.go#L676-L688

sudo-shashank commented 3 days ago

@elmattic topics fields in EthFilterSpec is a collection of EthHashList which is a collection of EthHash https://github.com/ChainSafe/forest/blob/a875e3dbdf4a5dec0a115614f7c6cad0c1d99626/src/rpc/methods/eth/types.rs#L351-L355

correct curl cmd:

curl -X POST 'http://127.0.0.1:2345/rpc/v1' \
    -H 'Content-Type: application/json' \
    --data '{"jsonrpc":"2.0","id":1,"method":"Filecoin.EthGetLogs","params":[{"blockHash": "0xa158a536fe66e18b8cbf5c392384d840483f84732e2a2ed8d9fb68cedcffef54", "address": ["0x51e1f72b655528de2d4d88e70bd53774db8d0b0c"], "topics": [["0x90890809c654f11d6e72a28fa60149770a0d11ec6c92319d6ceb2bb0a4ea1a15"]]}]}'
elmattic commented 3 days ago

Yes, that works, but a scalar is also possible instead of a list.