Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth_getFilterChanges returns a string containing serialized JSON array (not the JSON array) #640

Closed
peterbroadhurst opened this issue Jul 13, 2022 · 2 comments · Fixed by #641
Assignees
Labels
bug Something isn't working

Comments

@peterbroadhurst
Copy link
Contributor

peterbroadhurst commented Jul 13, 2022

eth_getFilterChanges returns a string containing serialized JSON array (not the JSON array)

Description

If you install a block filter, using eth_newBlockFilter, or a log filter, using eth_newFilter, then call eth_getFilterChanges you get a result such as:

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": "[\"0x3b6c51a0447f94cbcafa3cf8ae05721b972f18d6aafa2be69da31712f38699ef\",\"0x941de60877552e0559049d7e60ce53cbdffdf098ad039d786fa76aedbb63df26\",\"0x1cfcad6fa8a4d33d586817db2aac7ad7de8e4b825ba44a166f80f6d94d665661\",\"0xde6ccf731581c9b34901aea3487ec0b692bb17d3f2a3eebe99d824fb60c921bf\",\"0x73dcb8107af6d7bc882e528549f32c7f12f01fc0b8db116b92312fba75d72b65\",\"0x0d56fb2ba64770007132b1f56c662c547666735b6bebb2a9f8e521bfdd8d8e9d\"]"
}

You can see

Your environment

  • Edge version: v0.4.1
  • Dockerized linux (Ubuntu 18.04 base).

Steps to reproduce

For blocks

Request 1: eth_newBlockFilter

{
    "jsonrpc": "2.0",
    "id": "1",
    "method": "eth_newBlockFilter"
}

Response 1: eth_newBlockFilter

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": "2ee73d8b-5d37-46e1-b743-511b0e54a55d"
}

Request 2: eth_getFilterChanges

{
    "jsonrpc": "2.0",
    "id": "1",
    "method": "eth_getFilterChanges",
    "params": ["2ee73d8b-5d37-46e1-b743-511b0e54a55d"]
}

Response 2: eth_getFilterChanges

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": "[\"0x2e2fef24eea91c245e32cd2793b668a2b18b6202914e3e62e029709efca5c48c\"]"
}

For Logs

Request 1: eth_newFilter

{
    "jsonrpc": "2.0",
    "id": "1",
    "method": "eth_newFilter",
    "params": [
        {
            "fromBlock": "0",
            "topics": ["0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef"]
        }
    ]
}

Response 1:

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": "c2c92aa7-75ac-4d98-b392-00ccd10a4e73"
}

Request 2: eth_getFilterLogs - note this is GOOD

{
    "jsonrpc": "2.0",
    "id": "1",
    "method": "eth_getFilterLogs",
    "params": ["c2c92aa7-75ac-4d98-b392-00ccd10a4e73"]
}

Response 2: - note this is GOOD

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": [
        {
            "address": "0x432346fc656AbA8b1a4111374A9bDf56Df05d8d9",
            "topics": [
                "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
                "0x0000000000000000000000000000000000000000000000000000000000000000",
                "0x0000000000000000000000000151d512928dd4e1a0a0c9f3c125bf0380e6d587"
            ],
            "data": "0x0000000000000000000000000000000000000000033b2e3c9fd0803ce8000000",
            "blockNumber": "0x942",
            "transactionHash": "0xca1b4ff89454de929dfb1b12e264bfdedc9d09ef159b66b0699c4fe12952b945",
            "transactionIndex": "0x0",
            "blockHash": "0x41bf24b6a1d6f4af222558b57bf9b14759507dd75539e0bea8e6177242c955d2",
            "logIndex": "0x2",
            "removed": false
        }
    ]
}

Request 3: eth_getFilterChanges

{
    "jsonrpc": "2.0",
    "id": "1",
    "method": "eth_getFilterChanges",
    "params": ["07f14e93-7a10-446d-8c12-e08e482abc05"]
}

Response 3: - see here it is a string (not an array)

{
    "jsonrpc": "2.0",
    "id": "1",
    "result": "[{\"address\":\"0x432346fc656AbA8b1a4111374A9bDf56Df05d8d9\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",\"0x0000000000000000000000000000000000000000000000000000000000000000\",\"0x0000000000000000000000000151d512928dd4e1a0a0c9f3c125bf0380e6d587\"],\"data\":\"0x0000000000000000000000000000000000000000000000000000000000003039\",\"blockNumber\":\"0xa17\",\"transactionHash\":\"0x0be6eb34b8c03342dd8134fd22cbeaa2a0b27bbd3d8da4f6fcfe64cf624c7a29\",\"transactionIndex\":\"0x0\",\"blockHash\":\"0xe697e957bba8a04e79616dc266c683fbbed020caa501ac4f07e8ccae9b6a32b0\",\"logIndex\":\"0x0\",\"removed\":false}]"
}

Expected behaviour

The result should not be an array, not a string containing a serialized JSON array.

Actual behaviour

It's an array directly in the JSON/RPC result.

Logs

Easy to recreate.

Note: The unit test does not catch this, as the Go JSON/RPC wrapper used in the test masks this issue, by unmarshalling the array:
https://github.com/umbracle/ethgo/blob/8326bf607acf2ae4893128cacf205c67261cc411/jsonrpc/eth.go#L74-L86

Proposed solution

The problem appears to be specific to this area of code, because it's trying to handle either type of filter there could be two types of result.

// GetFilterChanges returns the updates of the filter with given ID in string
func (f *FilterManager) GetFilterChanges(id string) (string, error) {
f.lock.RLock()
defer f.lock.RUnlock()
filter, ok := f.filters[id]
if !ok {
return "", ErrFilterDoesNotExists
}
// we cannot get updates from a ws filter with getFilterChanges
if filter.isWS() {
return "", ErrWSFilterDoesNotSupportGetChanges
}
res, err := filter.getUpdates()
if err != nil {
return "", err
}
return res, nil
}

So I propose this code returns a json.RawMessage rather than string.

See discussion in #641 - this exposed a follow-on issue in the code building a JSON string array for empty arrays, here:

return fmt.Sprintf("[\"%s\"]", strings.Join(updates, "\",\"")), nil

It seems there's code up-stream that handles interface{} - so there's no need for this extra serialization step - so updating the PR for that.

I will work to provide that proposal in code via a PR, as a follow-up to this issue.

@0xAleksaOpacic
Copy link
Contributor

Hey @peterbroadhurst, thank you for opening this issue
I see that you've opened draft PR, if you need any help with it or you want us to take over the task please ask :)

@0xAleksaOpacic 0xAleksaOpacic added the bug fix Functionality that fixes a bug label Jul 15, 2022
@zivkovicmilos zivkovicmilos added bug Something isn't working and removed bug fix Functionality that fixes a bug labels Jul 19, 2022
@zivkovicmilos zivkovicmilos linked a pull request Jul 19, 2022 that will close this issue
11 tasks
@zivkovicmilos zivkovicmilos self-assigned this Jul 19, 2022
@peterbroadhurst
Copy link
Contributor Author

Sorry @Aleksao998 for the delay on this - latest updates added to PR: #641 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants