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

[v1.3] eth_newFilter incompatibility with eth #1025

Closed
2 of 5 tasks
gpBlockchain opened this issue Jul 21, 2022 · 13 comments
Closed
2 of 5 tasks

[v1.3] eth_newFilter incompatibility with eth #1025

gpBlockchain opened this issue Jul 21, 2022 · 13 comments
Assignees
Labels
bug Something isn't working P-Medium

Comments

@gpBlockchain
Copy link

gpBlockchain commented Jul 21, 2022

Env

url:https://godwoken-alphanet-v1.ckbapp.dev

incompatibility case

  • The second call eth_getFilterChanges should not have the data from the last call
       it("invoke eth_getFilterChanges 2 times, second logs length must be 0 ", async () => {
            const filterId = await ethers.provider.send("eth_newFilter", [{}]);

            await sendTxToAddBlockNum(3)
            let logs = await ethers.provider.send("eth_getFilterChanges", [filterId]);
            checkLogsIsSort(logs)
            logs = await ethers.provider.send("eth_getFilterChanges", [filterId]);
            expect(logs.toString()).to.be.equal('')
        })
  • filter : 'fromBlock': 'pending' ,should return invalid from and to block combination: from > to
guopenglin@192 axon % curl --location --request POST 'localhost:8545/' \
--header 'Content-Type: application/json' \
--data-raw '{
        "jsonrpc":"2.0",
        "method":"eth_newFilter",
        "params":[
                {
                        "fromBlock":"pending"
                }
        ],
        "id":73
}'
{"jsonrpc":"2.0","id":73,"error":{"code":-32000,"message":"invalid from and to block combination: from \u003e to"}}
guopenglin@192 axon % 
guopenglin@192 axon % 
guopenglin@192 axon % 
guopenglin@192 axon % curl --location --request POST 'https://godwoken-alphanet-v1.ckbapp.dev' \
--header 'Content-Type: application/json' \
--data-raw '{
        "jsonrpc":"2.0",
        "method":"eth_newFilter",
        "params":[
                {
                        "fromBlock":"pending"
                }
        ],
        "id":73
}'
{"jsonrpc":"2.0","id":73,"result":"0x046ed6136470c6ee84f9099feab6f25d"}%        
  • filter : fromBlock = eth_blockNumber()+1000 ,should filter log.number >= eth_blockNumber()+1000

  • filter : toBlock = earliest ,should return error msg

guopenglin@192 axon % curl --location --request POST 'https://godwoken-alphanet-v1.ckbapp.dev' \
--header 'Content-Type: application/json' \
--data-raw '{
        "jsonrpc":"2.0",
        "method":"eth_newFilter",
        "params":[
                {
                        "toBlock":"earliest"
                }
        ],
        "id":73
}'
{"jsonrpc":"2.0","id":73,"result":"0x8030aae6fbb589c1a4cd7475fa5d4a64"}%        guopenglin@192 axon % curl --location --request POST 'http://127.0.0.1:8545' \
--header 'Content-Type: application/json' \
--data-raw '{
        "jsonrpc":"2.0",
        "method":"eth_newFilter",
        "params":[
                {
                        "toBlock":"earliest"
                }
        ],
        "id":73
}'
{"jsonrpc":"2.0","id":73,"error":{"code":-32000,"message":"invalid from and to block combination: from \u003e to"}}
  • eth_getFilterChanges should not return 'Internal error'
jsonRpcRequest: {
  jsonrpc: '2.0',
  method: 'eth_getFilterChanges',
  params: [ '0x9c6fb86fd2fae1a9f1ea691dcab01980' ],
  id: 100
}
jsonRpcResponse: {
  jsonrpc: '2.0',
  id: 100,
  error: { code: -32603, message: 'Internal error' }
}

test code

         it("[[A, B], [A, B]].yes,should return logs", async () => {
                //check get filed id success
                expect(filterMsgMap["topic.[[A, B],[A, B]].yes"].error).to.be.equal(undefined)
                expect(filterMsgMap["topic.[[A, B],[A, B]].yes"].logs.length).to.be.not.equal(0)
                await checkLogsGteHeight(filterMsgMap["topic.[[A, B],[A, B]].yes"].logs, blockHeight)
                await checkLogsIsSort(filterMsgMap["topic.[[A, B],[A, B]].yes"].logs)
            })

test repo:https://github.com/gpBlockchain/godwoken-tests/blob/issue-test/contracts/test/Issue.js#L7

@keroro520
Copy link
Contributor

  • The second call eth_getFilterChanges should not have the data from the last call

Empty logs should be represented as [] but not ''.
The last assertion should be changed to

expect(logs).to.be.equal([]);

And I have created a similar testcase before 😄 https://github.com/nervosnetwork/godwoken-tests/pull/143/files#diff-5a9d016b1531b5de7f83f95c594c036c72e18a10fed58d5897157dce20beebbaR66

@keroro520
Copy link
Contributor

keroro520 commented Jul 25, 2022

  • filter : 'fromBlock': 'pending' ,should return invalid from and to block combination: from > to
  • filter : toBlock = earliest ,should return error msg

Your suggestion is a worthy enhancement. Currently, Godwoken-Web3 returns an empty array [], which is okay for users. Let's mark it as a P-Medium enhancement task.

@keroro520
Copy link
Contributor

  • filter : fromBlock = eth_blockNumber()+1000 ,should filter log.number >= eth_blockNumber()+1000
  • eth_getFilterChanges should not return 'Internal error'

I do not quite understand these two cases. Would you like to provide more information? @gpBlockchain

@gpBlockchain
Copy link
Author

gpBlockchain commented Jul 25, 2022

  • The second call eth_getFilterChanges should not have the data from the last call

Empty logs should be represented as [] but not ''. The last assertion should be changed to

expect(logs).to.be.equal([]);

And I have created a similar testcase before 😄 https://github.com/nervosnetwork/godwoken-tests/pull/143/files#diff-5a9d016b1531b5de7f83f95c594c036c72e18a10fed58d5897157dce20beebbaR66

[].toString() == ""
image
i will mod message that not use toString expected

@gpBlockchain
Copy link
Author

I do not quite understand these two cases. Would you like to provide more information

If the current block height < fromBlock, send eth_getFilterChanges should return empty ,but now will return logs that logs.height < fromBlock

@gpBlockchain
Copy link
Author

gpBlockchain commented Jul 25, 2022

I do not quite understand these two cases. Would you like to provide more information

  • filter : 'fromBlock': 'pending' ,should return invalid from and to block combination: from > to

If the current block height < fromBlock, send eth_getFilterChanges should return empty ,but now will return logs that logs.height < fromBlock

solidity code

function testLog4(uint256 logCount) public {
        for(uint256 i=0;i<logCount;i++){
            log4(
                bytes32(0x0000000000000000000000000000000000000000000000000000000000000000),
                bytes32(0x0000000000000000000000000000000000000000000000000000000000000001),
                bytes32(0x0000000000000000000000000000000000000000000000000000000000000002),
                bytes32(0x0000000000000000000000000000000000000000000000000000000000000003),
                bytes32(0x0000000000000000000000000000000000000000000000000000000000000004)
            );
        }
    }

constant

let topic0="0x0000000000000000000000000000000000000000000000000000000000000001";
let topic1="0x0000000000000000000000000000000000000000000000000000000000000002";
let topic2="0x0000000000000000000000000000000000000000000000000000000000000003";
let topic3="0x0000000000000000000000000000000000000000000000000000000000000004";

step

  1. invoke deploy contract that contains testLog4 method
  2. send eth_newFillter that fillter is {"topics": [[topic0, topic2,topic3], [null, topic2],[topic1]] }
  3. invoke testLog4 method 20 times
  4. send eth_getFilterChanges

result

  1. eth_getFilterChanges return 'Internal error' expected return log message

@keroro520
Copy link
Contributor

"topics": [[topic0, topic2,topic3], [null, topic2],[topic1]] is invalid as subtopic cannot be null.

@keroro520
Copy link
Contributor

"topics": [[topic0, topic2,topic3], [null, topic2],[topic1]] is invalid as subtopic cannot be null.

It should throw InvalidFilterTopic when creating a filter, but now InternalError. It is a bug.

@Flouse Flouse added the bug Something isn't working label Aug 4, 2022
@Flouse Flouse assigned classicalliu and unassigned keroro520 Jan 10, 2023
@Flouse Flouse transferred this issue from godwokenrises/godwoken-web3 Mar 20, 2023
@classicalliu
Copy link
Contributor

classicalliu commented Mar 20, 2023

  1. It's passed on testnet & alphanet
  2. It returns {"jsonrpc":"2.0","id":73,"result":"0x1"} on my http://localhost:8545(hardhat node)
  3. This test case will also failed in hardhat, from hardhat's code , when fromBlock > latestBlockNumberfromBlock will set to latestBlockNumber
  4. It returns {"jsonrpc":"2.0","id":73,"result":"0x2"} on hardhat node
  5. It will be fixed by fix(web3): Topic includes null #1024

@sunchengzhu
Copy link
Collaborator

  1. I use eth_newFilter in alchemy's goerli network, which supports the use of fromBlock greater than the current block height, and the filter will take effect when the block height reaches fromBlock.
    And the same scenario on Godwoken alphanet v1.13.0-rc1 eth_getFilterChange returns {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"header not found"}}.

@sunchengzhu
Copy link
Collaborator

sunchengzhu commented Apr 7, 2023

I found that the first call of eth_getFilterChanges is different in hardhat and geth. Is godwoken consistent with hardhat or geth?

  1. Use "fromBlock": "0x1" to create a filter
  2. Pass in the created filter_id to call eth_getFilterChanges
  3. Hardhat will return all log arrays from 0x1 to the current block matching, while geth will only return[]
    image

@classicalliu
Copy link
Contributor

  1. I use eth_newFilter in alchemy's goerli network, which supports the use of fromBlock greater than the current block height, and the filter will take effect when the block height reaches fromBlock.
    And the same scenario on Godwoken alphanet v1.13.0-rc1 eth_getFilterChange returns {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"header not found"}}.

Performs like Geth now

@classicalliu
Copy link
Contributor

I found that the first call of eth_getFilterChanges is different in hardhat and geth. Is godwoken consistent with hardhat or geth?

  1. Use "fromBlock": "0x1" to create a filter
  2. Pass in the created filter_id to call eth_getFilterChanges
  3. Hardhat will return all log arrays from 0x1 to the current block matching, while geth will only return[]
    image

We can keep the status quo, which performs like Geth now.

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

No branches or pull requests

5 participants