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

Effective gas price #4759

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Effective gas price #4759

wants to merge 13 commits into from

Conversation

Frozen
Copy link
Contributor

@Frozen Frozen commented Sep 17, 2024

This PR address this issue by setting the effectiveGasPrice to the transaction gas price. It is not implementing the full EIP 1559.

EffectiveGasPricefield added to TxRusult. Added json and rlp serialization tests.

@Frozen Frozen self-assigned this Sep 17, 2024
@Frozen Frozen force-pushed the feature/effective-gas-price branch from 9ddc0cd to c6faf4c Compare September 17, 2024 20:44
@Frozen Frozen marked this pull request as ready for review September 20, 2024 16:43
@Frozen Frozen force-pushed the feature/effective-gas-price branch from 0bb2b32 to e3a5dd6 Compare September 20, 2024 17:40
rpc/harmony/v2/types.go Outdated Show resolved Hide resolved
@sophoah
Copy link
Contributor

sophoah commented Sep 23, 2024

@Frozen During my test, effectiveGasPrice field in the transaction receipt is still not available. I checked eth_getTransactionReceipt and hmy_getTransactionReceipt

Please make sure the effectiveGasPrice is the same as the gasPrice

@Frozen Frozen force-pushed the feature/effective-gas-price branch 2 times, most recently from a230a06 to af35364 Compare September 27, 2024 21:10
@sophoah
Copy link
Contributor

sophoah commented Sep 30, 2024

@Frozen hmyv1 and v2 APIs has the hex vs decimal difference see below effectiveGasPrice in decimal vs cumulatedGasPrice in hex

{
  "transaction-hash": "0x6ce7fd454c5c3e642a4573bd684f28acaf5f5478dddf56f1ec64c8808b57190a",
  "blockchain-receipt": {
    "blockHash": "0xfacba8e14ccd7ae0e066df602b49251a09bccd510597fa8f5c17b0859df493f0",
    "blockNumber": "0x9",
    "contractAddress": "0x0000000000000000000000000000000000000000",
    "cumulativeGasUsed": "0x5208",
    "effectiveGasPrice": 100000000000,
    "from": "one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur",
    "gasUsed": "0x5208",
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "root": "0x",
    "shardID": 0,
    "status": "0x1",
    "to": "one19zzwsxr0uf2fe34y8qkadek2v0eh6h5mg2deg6",
    "transactionHash": "0x6ce7fd454c5c3e642a4573bd684f28acaf5f5478dddf56f1ec64c8808b57190a",
    "transactionIndex": "0x0"
  },
  "time-signed-utc": "2024-09-30 08:17:36.579582"
}

Also eth_getTransactionReceipt still doesn't have the field effectiveGasPrice

{
 "jsonrpc": "2.0",
 "id": 1,
 "result": {
   "blockHash": "0xfacba8e14ccd7ae0e066df602b49251a09bccd510597fa8f5c17b0859df493f0",
   "blockNumber": "0x9",
   "contractAddress": null,
   "cumulativeGasUsed": "0x5208",
   "from": "0x15a128e599b74842bccba860311efa92991bffb5",
   "gasUsed": "0x5208",
   "logs": [],
   "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
   "status": "0x1",
   "to": "0x2884e8186fe2549cc6a4382dd6e6ca63f37d5e9b",
   "transactionHash": "0xad611cd8342d244e74f64888309fdf819c23f308f425edbeee289492729ae47a",
   "transactionIndex": "0x0"
 }
}

@Frozen Frozen force-pushed the feature/effective-gas-price branch from af35364 to 150f2f5 Compare October 14, 2024 22:22
@Frozen
Copy link
Contributor Author

Frozen commented Oct 16, 2024

Fixed json hex view for effectiveGasPrice

  "cumulativeGasUsed": "0x5208",
  "effectiveGasPrice": "0x17480",

@sophoah
Copy link
Contributor

sophoah commented Oct 17, 2024

eth_getTransactionReceipt and the blockchain receipt generated by the hmy command during a transfer works now.

Could you check hmy_getTransactionReceipt and hmyv2_getTransactionReceipt ? those would be the last ones that should require the field effectiveGasPrice

@ChiTimesChi
Copy link

@sophoah @Frozen is there an ETA for this PR to be completed? This is causing issues when trying to use foundry to interact with the harmony blockchain. It is my understanding that EffectiveGasPrice is a mandatory field in the latest spec.

foundry-rs/foundry#7640 (comment)

@Frozen
Copy link
Contributor Author

Frozen commented Jan 8, 2025

it will be included in next release

@Frozen Frozen force-pushed the feature/effective-gas-price branch from 302422e to d308ae9 Compare January 9, 2025 03:49
@sophoah
Copy link
Contributor

sophoah commented Jan 10, 2025

@Frozen hmy_getTransactionReceipt and hmy_getTransactionReceipt are good :
hmy:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "blockHash": "0x29e06a818eb40489e346b41717f4b4f314ec06a67d9cdb7e0f721be8667cc402",
    "blockNumber": "0x3",
    "contractAddress": "0x0000000000000000000000000000000000000000",
    "cumulativeGasUsed": "0x5208",
    "effectiveGasPrice": "0x174876e800",
    "from": "one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur",
    "gasUsed": "0x5208",
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "root": "0x",
    "shardID": 0,
    "status": "0x1",
    "to": "one1eenp9ujcrmyaq22ef6jrpry2k97tjz4xs6ppcf",
    "transactionHash": "0x54e5a487016f05fa402b314c1e1aac397163863842c1914c3a3511a77e0cc838",
    "transactionIndex": "0x0"
  }
}

eth:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "blockHash": "0x29e06a818eb40489e346b41717f4b4f314ec06a67d9cdb7e0f721be8667cc402",
    "blockNumber": "0x3",
    "contractAddress": null,
    "cumulativeGasUsed": "0x5208",
    "effectiveGasPrice": "0x174876e800",
    "from": "0x15a128e599b74842bccba860311efa92991bffb5",
    "gasUsed": "0x5208",
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "status": "0x1",
    "to": "0xce6612f2581ec9d029594ea4308c8ab17cb90aa6",
    "transactionHash": "0x50fbac1de6df537d569cd24f962f08986800bcd3939a41144a072b0dcb6a7674",
    "transactionIndex": "0x0"
  }
}

however hmyv2_getTransactionReceipt should return the result in decimal (non hex), it currently return an hex, see cumulativeGasUsed, gasUsed are in decimal below except effectiveGasPrice:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "blockHash": "0x29e06a818eb40489e346b41717f4b4f314ec06a67d9cdb7e0f721be8667cc402",
    "blockNumber": 3,
    "contractAddress": "0x0000000000000000000000000000000000000000",
    "cumulativeGasUsed": 21000,
    "effectiveGasPrice": "0x174876e800",
    "from": "one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur",
    "gasUsed": 21000,
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "root": "0x",
    "shardID": 0,
    "status": 1,
    "to": "one1eenp9ujcrmyaq22ef6jrpry2k97tjz4xs6ppcf",
    "transactionHash": "0x54e5a487016f05fa402b314c1e1aac397163863842c1914c3a3511a77e0cc838",
    "transactionIndex": 0
  }
}

@sophoah
Copy link
Contributor

sophoah commented Jan 13, 2025

hey @GheisMohammadi please review/approve this PR

@sophoah
Copy link
Contributor

sophoah commented Jan 13, 2025

@Frozen thanks hmyv2 looks good now

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "blockHash": "0x896cfee82ef9542433cdf392548ce519bb7d531eec9d018f26b39e29f0e055b0",
    "blockNumber": 9,
    "contractAddress": "0x0000000000000000000000000000000000000000",
    "cumulativeGasUsed": 21000,
    "effectiveGasPrice": 100000000000,
    "from": "one1zksj3evekayy90xt4psrz8h6j2v3hla4qwz4ur",
    "gasUsed": 21000,
    "logs": [],
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "root": "0x",
    "shardID": 0,
    "status": 1,
    "to": "one19zzwsxr0uf2fe34y8qkadek2v0eh6h5mg2deg6",
    "transactionHash": "0x6ce7fd454c5c3e642a4573bd684f28acaf5f5478dddf56f1ec64c8808b57190a",
    "transactionIndex": 0
  }
}

.travis.yml Outdated Show resolved Hide resolved
@sophoah
Copy link
Contributor

sophoah commented Jan 13, 2025

hey @sunwavesun can you help review/approve the PR ?

@sophoah
Copy link
Contributor

sophoah commented Jan 17, 2025

@mur-me we talked about the ci test failure in our previous meeting

I've confirmed that our localnet is getting the effectiveGasPrice from transactionReceipt RPC : https://github.com/harmony-one/go-sdk/blob/master/cmd/subcommands/staking.go#L174 so the current branch on harmony-test is wrong like you said.

@mur-me
Copy link
Collaborator

mur-me commented Jan 17, 2025

RPC tests were updated, I'm trying to rerun them

Link to the harmony-test PR - harmony-one/harmony-test#34

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

Successfully merging this pull request may close these issues.

5 participants