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

[rpc] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk #3826

Closed
wants to merge 1 commit into from

Conversation

AlexiaChen
Copy link
Contributor

@AlexiaChen AlexiaChen commented Jul 14, 2021

Issue

#3821

Test

No test with @hypnagonia

Local Test

  • tested hmy
  • tested hmyv2
  • tested eth

Use RPC to comapre reponse data for same blockHash and TxHash is consistent:

curl -d '{ "jsonrpc":"2.0", "method":"hmy_getBlockByHash", "params":[ "0x73852a261acd91e6ee0b72e4b39eb3ee42ec31cfb13a5b1b0fdb676e9dbc358c", true], "id":1 }' -H "Content-Type:application/json" -X POST http://127.0.0.1:9500

response data:

data.zip

curl -d '{ "jsonrpc":"2.0", "method":"hmy_getReceiptsByBlockHash", "params":["0x73852a261acd91e6ee0b72e4b39eb3ee42ec31cfb13a5b1b0fdb676e9dbc358c"], "id":1 }' -H "Content-Type:application/json" -X POST http://127.0.0.1:9500

reponse data:

bb.zip

curl -d '{ "jsonrpc": "2.0", "method": "hmy_getTransactionReceipt", "params": [ "0xcd2634fdaaff70ab8c538e99b8cbf44ee56fd2073f54ceeeeeb9962b447691a8"], "id": 1}' -H "Content-Type:application/json" -X POST http: //127.0.0.1:9500

reponse data:

uu.zip

@AlexiaChen AlexiaChen added block-explorer-backend block-explorer-backend related issues rpc RPC or API labels Jul 14, 2021
@AlexiaChen AlexiaChen self-assigned this Jul 14, 2021
@AlexiaChen AlexiaChen force-pushed the improve-used-gas branch 2 times, most recently from 03fe3aa to d988736 Compare July 14, 2021 09:39
Copy link
Contributor

@JackyWYX JackyWYX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very sure about this design, but why can't we simply use Receipt.GasUsed? Is there anything missing here?

@AlexiaChen
Copy link
Contributor Author

AlexiaChen commented Jul 15, 2021

@JackyWYX

From @hypnagonia 's description, the explorer usually calls RPC GetBlocks, and GetBlocks calls GetBlockByNumber one by one from start height to end height. the information returned here does not have the usedGas of each tx. maybe @hypnagonia needs to get each tx directly from the return message here And indeed there is no similar interface like GetReceiptByTxHash, only GetReceiptsByBlockHash

Copy link
Contributor

@JackyWYX JackyWYX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand whether there is need to modify APIs. This operation will add some extra burden to already heavily used GetBlockByNumber. Would you please add some lines of description of the necessity of modifying APIs instead of optimizing at DAPP?

@AlexiaChen
Copy link
Contributor Author

@JackyWYX

Do you mean to add a new RPC to serve the query usedGas? Instead of putting it inside the return body of GetBlockByNumber improve payload pressure?

Actually, here's what I was thinking

  • First, @hypnagonia 's original requirement is to put it in GetBlockByNumber(maybe this is necessity for explorer? ), maybe this is more convenient for his business processing, because adding FullTx will get the usedGas list directly, adding RPC will have more changes to his explorer business code

  • Second, is it really necessary to add an RPC to query the usedGas list? Because the usedGas payload is just transferred to this new RPC, the pressure on the chain's own requests may not change, because the usedGas list should not occupy much, after all, the FullTx parameters returned payload should not be small, even if the usedGas list of payload is also irrelevant

Perhaps, you have other better suggestions?

@JackyWYX
Copy link
Contributor

Perhaps, you have other better suggestions?

Why don't you implement a new RPC method called GetReceiptsByBlockHash?

@AlexiaChen
Copy link
Contributor Author

Perhaps, you have other better suggestions?

Why don't you implement a new RPC method called GetReceiptsByBlockHash?

As I mentioned above, adding a new RPC is indeed one way to do this. So it mostly depends on @hypnagonia 's needs. If he feels that this approach will not cause too many changes to his business code, I can do that.

@AlexiaChen AlexiaChen changed the title [DO NOT MERGE] [WIP] [explorer] Support query usedGas for each tx for explorer node [explorer] Support query usedGas for each tx for explorer node Jul 19, 2021
@AlexiaChen AlexiaChen changed the title [explorer] Support query usedGas for each tx for explorer node [explorer] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk Jul 19, 2021
@AlexiaChen AlexiaChen requested review from rlan35 and LeoHChen July 19, 2021 13:53
@AlexiaChen AlexiaChen changed the title [explorer] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk [rpc] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk Jul 19, 2021
@AlexiaChen AlexiaChen changed the title [rpc] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk [rpc] [WIP] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk Jul 20, 2021
@AlexiaChen AlexiaChen changed the title [rpc] [WIP] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk [rpc] Add New RPC GetReceiptsByBlockHash for quering all receipts of the block in bulk Jul 20, 2021
@AlexiaChen
Copy link
Contributor Author

Hi,@JackyWYX could you pleaese review it again?

@JasonTy
Copy link

JasonTy commented Nov 24, 2021

Hi, Did the gas returned through hmy_getTransactionReceipt experience hex twice?

@AlexiaChen AlexiaChen closed this Jan 22, 2022
@AlexiaChen AlexiaChen deleted the improve-used-gas branch January 22, 2022 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
block-explorer-backend block-explorer-backend related issues rpc RPC or API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants