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_getblockbynumber supports finalized except latest, pending and blocknumber #5953

Open
waynercheung opened this issue Aug 14, 2024 · 15 comments · May be fixed by #6007
Open

eth_getblockbynumber supports finalized except latest, pending and blocknumber #5953

waynercheung opened this issue Aug 14, 2024 · 15 comments · May be fixed by #6007

Comments

@waynercheung
Copy link
Contributor

waynercheung commented Aug 14, 2024

Background

I need to get the latest solidified/finalized block number through jsonrpc, but this method doesn't support yet.

Rationale

Why should this feature exist?
For many developers, they need to get the latest solidified/finalized block number, then fetch other data by this number.

What are the use-cases?
For example, I want to fetch the specific events by eth_getLogs, I should get the latest solidified/finalized block number, then using the eth_getLogs to fetch the logs.

Specification

The api eth_getBlockByNumber, eth_getBlockTransactionCountByNumber and eth_getLogs will support the parameter finalized, treating it the same as the solid block in java-tron.
The other jsonrpc api will not support this parameter, just returns message as TAG [earliest | pending | finalized] not supported.

Test Specification

Scope Of Impact

Implementation

Do you have ideas regarding the implementation of this feature?
Yes.
Actually, we can get the current solidified block number by chainBaseManager.getDynamicPropertiesStore().getLatestSolidifiedBlockNum();.

Are you willing to implement this feature?
Yes

@tomatoishealthy
Copy link
Contributor

Sounds great.

This title is a bit confusing to me, I understand what you are going to do after reading the content.

Will you implement it on the existing interface by adding new parameters? Or if you are adding a new interface, can you describe it like this: Add a new JSON-RPC API get_ finalizedblocknumber to return the current solidity block number

@317787106
Copy link
Contributor

317787106 commented Aug 15, 2024

@tomatoishealthy I think it doesn't add a new JSON-RPC API but eth_getBlockByNumber support new String parameter finalized except three kind like this:

  public Block getByJsonBlockId(String id) throws JsonRpcInvalidParamsException {
    if (EARLIEST_STR.equalsIgnoreCase(id)) {
      return getBlockByNum(0);
    } else if ("latest".equalsIgnoreCase(id)) {
      return getNowBlock();
    } else if ("pending".equalsIgnoreCase(id)) {
      throw new JsonRpcInvalidParamsException("TAG pending not supported");
    } else {
      long blockNumber;
      try {
        blockNumber = ByteArray.hexToBigInteger(id).longValue();
      } catch (Exception e) {
        throw new JsonRpcInvalidParamsException("invalid block number");
      }

      return getBlockByNum(blockNumber);
    }
  }

These are 3 kinds:

  public static final String EARLIEST_STR = "earliest";
  public static final String PENDING_STR = "pending";
  public static final String LATEST_STR = "latest";

@317787106
Copy link
Contributor

@zhangwenhua-tron Is other jsonrpc api should also support finalized parameter?

@waynercheung
Copy link
Contributor Author

waynercheung commented Aug 16, 2024

@waynercheung Is other jsonrpc api should also support finalized parameter?

I think make eth_getBlockByNumber support finalized parameter has a higher priority than other api, because fetching the latest finalized block is needed for many developers.

About other api, maybe we can support it later.

@317787106
Copy link
Contributor

317787106 commented Aug 16, 2024

@zhangwenhua-tron Why not use the httpSolidityPort? eth_getBlockByNumber("latest") in solidity may return the finalized block number.

  jsonrpc {
    # Note: If you turn on jsonrpc and run it for a while and then turn it off, you will not
    # be able to get the data from eth_getLogs for that period of time.

    # httpFullNodeEnable = true
    # httpFullNodePort = 8545
    httpSolidityEnable = true
    httpSolidityPort = 8555
    # httpPBFTEnable = true
    # httpPBFTPort = 8565
  }

@tomatoishealthy
Copy link
Contributor

@zhangwenhua-tron Would it be convenient to introduce this issue at the next Core Devs Community Call 22?

@waynercheung
Copy link
Contributor Author

waynercheung commented Aug 16, 2024

@317787106
For the single fullnode, the user can simply use either 8545 or 8555 port, just as the same as the wallet/XXX or walletsolidity/XXX uri, but for trongrid(https://api.trongrid.io/jsonrpc), it only supports the 8545.

Beside, for the developer from the ETH or BSC, they maybe have no idea what is the difference between wallet and walletsolidity. So support for eth_getBlockByNumber(“finalized”) is necessary for better compatibility with EVM jsonrpc.

@abn2357
Copy link

abn2357 commented Aug 19, 2024

@zhangwenhua-tron Is other jsonrpc api should also support finalized parameter?

I think make eth_getBlockByNumber support finalized parameter has a higher priority than other api, because fetching the latest finalized block is needed for many developers.

About other api, maybe we can support it later.

@zhangwenhua-tron I think supporting finalized parameter in eth_getBlockByNumber will be confusing, the title of the function is byNumber, but the parameter is "finalized"

@waynercheung
Copy link
Contributor Author

waynercheung commented Aug 22, 2024

@waynercheung I think supporting finalized parameter in eth_getBlockByNumber will be confusing, the title of the function is byNumber, but the parameter is "finalized"

@abn2357
hi, you can get the spec from https://docs.infura.io/api/networks/ethereum/json-rpc-methods/eth_getblockbynumber.

@abn2357
Copy link

abn2357 commented Aug 23, 2024

@zhangwenhua-tron I think supporting finalized parameter in eth_getBlockByNumber will be confusing, the title of the function is byNumber, but the parameter is "finalized"

@abn2357 hi, you can get the spec from https://docs.infura.io/api/networks/ethereum/json-rpc-methods/eth_getblockbynumber.

@zhangwenhua-tron In the referenced doc, finalized block means latest finalized block, the additional information 'latest' can't be obtained through 'finalized' . I still think it is not a good api(name).

@waynercheung
Copy link
Contributor Author

waynercheung commented Aug 23, 2024

@waynercheung In the referenced doc, finalized block means latest finalized block, the additional information 'latest' can't be obtained through 'finalized' . I still think it is not a good api(name).

For EVM, finalized is supported after the Merge, it actually has the same meaning as solidity in TRON.

For eth_getBlockByNumber , it is the api spec from EVM, not for the TRON itself.

What we talking about, is not the name, it is the compatibility with EVM jsonrpc.

Besides, if you have better idea about the api(name), you can submit issue for https://github.com/ethereum/execution-apis or https://ethereum.github.io/execution-apis/api-documentation/.

@waynercheung
Copy link
Contributor Author

Hi, I have made a PR as #6007

@lvs007 lvs007 linked a pull request Sep 19, 2024 that will close this issue
@317787106
Copy link
Contributor

317787106 commented Nov 1, 2024

@waynercheung Any other methods handled EARLIEST_STR, PENDING_STR, LATEST_STR should also handle finalized parameter.

@waynercheung
Copy link
Contributor Author

@waynercheung Any other methods handled EARLIEST_STR, PENDING_STR, LATEST_STR should also handle finalized parameter.

OK

@waynercheung
Copy link
Contributor Author

@waynercheung Any other methods handled EARLIEST_STR, PENDING_STR, LATEST_STR should also handle finalized parameter.

Hi @317787106 , I have updated the codes and you can review it now.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants