godwokenrises / godwoken-web3

Moved to monorepo https://github.com/godwokenrises/godwoken/tree/develop/web3
14 stars 17 forks source link

eth_getLogs - null should be accepted as valid topic #380

Closed e00dan closed 2 years ago

e00dan commented 2 years ago

null should be accepted as valid entry in topics parameter in eth_getLogs request. On Rinkeby it works, on Godwoken Testnet v1.1 it doesn't work at this moment and Gnosis Safe Indexer can't work properly because of this.

Check Ethereum RPC specification, null is accepted: image

https://playground.open-rpc.org/?schemaUrl=https://raw.githubusercontent.com/ethereum/execution-apis/assembled-spec/openrpc.json&uiSchema%5BappBar%5D%5Bui:splitView%5D=false&uiSchema%5BappBar%5D%5Bui:input%5D=false&uiSchema%5BappBar%5D%5Bui:examplesDropdown%5D=false

I think null in topics works as specified here:

If you want to skip searching for the first topic and search for the second you need to specify null for the first topic. This causes the node to skip the first topic.

Source: https://github.com/ethereum/go-ethereum/issues/2085#issuecomment-166259735

Request:

{
    "jsonrpc": "2.0",
    "method": "eth_getLogs",
    "params": [
        {
            "fromBlock": "0xb3c3",
            "toBlock": "0xb3c4",
            "address": "0x19cc263d722a21f5e373056cf74a6239f72b0bfe",
            "topics": ["0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", null, ["0x000000000000000000000000f2b8260a9e42a50d46145715b94ab0613e76ec44"]]
        }
    ],
    "id": 0
}

Expected: Array of events or empty array is returned.

Actual: Error is returned.

{
    "jsonrpc": "2.0",
    "id": 0,
    "error": {
        "code": -32602,
        "message": "invalid argument 0: filter topic[] Array -> topic string -> hexString argument must be a string type"
    }
}
e00dan commented 2 years ago

I see this is already addressed by this PR: https://github.com/nervosnetwork/godwoken-web3/pull/375

I didn't notice it before opening an issue.

RetricSu commented 2 years ago

testnet has been updated, can you confirm this issue can be closed? @Kuzirashi

e00dan commented 2 years ago

@RetricSu

It seems fixed. Thank you.

Gnosis Safe Indexer seems to work now: image