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

[chain] Gas Estimations #1826

Open
zivkovicmilos opened this issue Mar 25, 2024 · 4 comments
Open

[chain] Gas Estimations #1826

zivkovicmilos opened this issue Mar 25, 2024 · 4 comments
Assignees
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related

Comments

@zivkovicmilos
Copy link
Member

Description

This effort covers the introduction of gas estimation functionality for transactions.

Copied from an issue on the Gno repo:
For users to send transactions that will eventually be committed to the Gno chain, they essentially need to provided 2 pieces of information gas-wise:

  • Gas Limit - the amount of gas (units of gas) the user is willing to pay for a given transaction to go through
  • Gas Price - the amount of native currency the user is willing to pay for a single unit of gas

Currently, gas estimation is not possible for a client application interacting with gno, and therefore applications and users need to guess the gas limit. If they guess incorrectly (too low), their transaction is rejected as being underpriced.
Additionally, there is the problem of gas fees being fixed to 1GNOT (1000000ugnot), as outlined in issue #649.

Ethereum has support for gas estimations with the following RPC endpoints:

  • eth_estimateGas - returns the lowest gas limit at which the transaction is successful. It finds the lowest value by doing a binary search over the gas range, and executing the transaction for each (middle) value in that range
  • eth_gasPrice - returns the current average gas price for a (any) transaction, in wei. Usually, this average is implemented as a rolling average value, and is tied temporally to the node serving the call (the average is reset when the node is restarted)

Successful outcome of this effort:

  • A user can estimate the minimum amount of gas their transaction will use
  • A user can see the current average gas price for the network
@thehowl
Copy link
Member

thehowl commented May 7, 2024

Note that at the time of writing, -gas-fee is completely ignored; you can even set it to 1ugnot. the keeper will detract 1GNOT independently of what is passed to -gas-fee (ideally, I'd say, keeper calculates a minimum gas fee for the given gas wanted, checks it against gas-fee, then detracts the value of gas-fee -- this AFAIK is part of the work being undertaken by @piux2 @deelawn)

The reason I'm writing here, though, is relating to the latest work undertaken in #1702, which improves the UX for simulating transactions on gnokey.

it seems to me that the approach you quoted @zivkovicmilos with eth_estimateGas will work well once there is enough historical data for a transaction; and one which can probably also better be provided by a transaction indexer rather than the main node, what do you think?

(I think we can still reasonably provide something like eth_gasPrice, which maps a unit of "gas" into the corresponding GNOT fee based on network congestion?)

For what concerns test4; after #1702 is merged, I propose setting up a "gas estimation system" which would work as follows:

  • the -gas-wanted parameter of gnokey accepts a new format: auto and auto+<integer>, and to auto+10000 by default
    • this works only when the -simulate parameter is set to test
    • the transaction simulation is made by making a transaction with the -gas-limit set to the max gas per block (currently 10M), and executed on the node
    • auto is substituted with the amount of used gas in the simulation, and added with the value of <integer> if given
  • the -gas-fee parameter of gnokey is set to 1ugnot for now
    • but eventually, once the chain actually respects this, it can be set to the value of gas wanted * gasPrice?

@zivkovicmilos
Copy link
Member Author

@thehowl
I like the syntax suggestion for gnokey, I think it's gonna bump UX a lot

We need to have an estimation endpoint on the node and not (just) on the indexer because we actually want to "simulate" transactions which are unsigned, and have no specified gas values yet.
The flow here is:
client creates tx -> estimates gas -> sends transaction to node

Gas price delegation to the indexer makes sense, since this value is actually historical, while the estimation depends on the VM

@Kouteki
Copy link
Contributor

Kouteki commented Oct 16, 2024

To be clear, the output of this effort is an endpoint that the user can call?

@Kouteki Kouteki moved this to Onbloc (unconfirmed) in 🍜 Seoul triage Nov 13, 2024
@Kouteki
Copy link
Contributor

Kouteki commented Nov 18, 2024

Relevant PR

#2838

@Kouteki Kouteki moved this from Onbloc (unconfirmed) to Onbloc (confirmed) in 🍜 Seoul triage Nov 18, 2024
@dongwon8247 dongwon8247 moved this to In Progress in 🤝🏻 Partner: Onbloc Nov 27, 2024
@Kouteki Kouteki moved this from Todo to In Progress in 🧙‍♂️gno.land core team Nov 27, 2024
@Kouteki Kouteki added 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Nov 27, 2024
zivkovicmilos added a commit that referenced this issue Dec 4, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->
## Descriptions

To simulate transactions, utilize the `.app/simulate` method for ABCI
Query.

### Changes
1. change the path of ABCI Query's `.app` to the result data storage
location.
- You can receive the query result data as
`RequestQuery.ResponseData.Data` instead of `RequestQuery.Value`.
- Provide it in a common form with other ABCI Queries.
2. remove the gas-consume logic of mocking signature data that is
executed when simulating transactions
([/tm2/pkg/sdk/auth/ante.go#L231-L237](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L231-L237))
- We will get the correct value when simulating a real transaction.
- We want transactions to run without signatures, but we already have
checks in place to see if a signature exists.
([tm2/pkg/sdk/auth/ante.go#L104-L106](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L104-L106))

### Example
#### [Request Simulate]
```curl
curl --location 'http://localhost:26657' \
--header 'Content-Type: application/json' \
--data '{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "abci_query",
  "params": [
    ".app/simulate",
    "CnMKDS9iYW5rLk1zZ1NlbmQSYgooZzFqZzhtdHV0dTlraGhmd2M0bnhtdWhjcGZ0ZjBwYWpkaGZ2c3FmNRIoZzFmZnp4aGE1N2RoMHFndjltYTV2MzkzdXIwemV4ZnZwNmxzanBhZRoMNTAwMDAwMHVnbm90Eg4IgIl6EggzMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohA+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2yEkCrIOTBt7YcDGcQ6Ohfv1r3nftAPaTATAtPfYD5zLQf7WDf1KPvWARe//CANtLLtIzcPVl7P/HnHxmfCYEwfGogIgUxMjMxMw==", 
    "0", 
    false
  ]
}'
```

#### [Response]
```curl
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "response": {
      "ResponseBase": {
        "Error": null,
        "Data": "eyJFcnJvciI6bnVsbCwiRGF0YSI6IiIsIkV2ZW50cyI6W10sIkxvZyI6Im1zZzowLHN1Y2Nlc3M6dHJ1ZSxsb2c6LGV2ZW50czpbXSIsIkluZm8iOiIiLCJHYXNXYW50ZWQiOjEwMDAwMDAsIkdhc1VzZWQiOjQ0NjI5fQ==",
        "Events": null,
        "Log": "",
        "Info": ""
      },
      "Key": null,
      "Value": null,
      "Proof": null,
      "Height": "0"
    }
  }
}
```

### Related Issue
- #1826

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: n3wbie <[email protected]>
Co-authored-by: Miloš Živković <[email protected]>
omarsy pushed a commit to TERITORI/gno that referenced this issue Dec 7, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->
## Descriptions

To simulate transactions, utilize the `.app/simulate` method for ABCI
Query.

### Changes
1. change the path of ABCI Query's `.app` to the result data storage
location.
- You can receive the query result data as
`RequestQuery.ResponseData.Data` instead of `RequestQuery.Value`.
- Provide it in a common form with other ABCI Queries.
2. remove the gas-consume logic of mocking signature data that is
executed when simulating transactions
([/tm2/pkg/sdk/auth/ante.go#L231-L237](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L231-L237))
- We will get the correct value when simulating a real transaction.
- We want transactions to run without signatures, but we already have
checks in place to see if a signature exists.
([tm2/pkg/sdk/auth/ante.go#L104-L106](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L104-L106))

### Example
#### [Request Simulate]
```curl
curl --location 'http://localhost:26657' \
--header 'Content-Type: application/json' \
--data '{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "abci_query",
  "params": [
    ".app/simulate",
    "CnMKDS9iYW5rLk1zZ1NlbmQSYgooZzFqZzhtdHV0dTlraGhmd2M0bnhtdWhjcGZ0ZjBwYWpkaGZ2c3FmNRIoZzFmZnp4aGE1N2RoMHFndjltYTV2MzkzdXIwemV4ZnZwNmxzanBhZRoMNTAwMDAwMHVnbm90Eg4IgIl6EggzMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohA+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2yEkCrIOTBt7YcDGcQ6Ohfv1r3nftAPaTATAtPfYD5zLQf7WDf1KPvWARe//CANtLLtIzcPVl7P/HnHxmfCYEwfGogIgUxMjMxMw==", 
    "0", 
    false
  ]
}'
```

#### [Response]
```curl
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "response": {
      "ResponseBase": {
        "Error": null,
        "Data": "eyJFcnJvciI6bnVsbCwiRGF0YSI6IiIsIkV2ZW50cyI6W10sIkxvZyI6Im1zZzowLHN1Y2Nlc3M6dHJ1ZSxsb2c6LGV2ZW50czpbXSIsIkluZm8iOiIiLCJHYXNXYW50ZWQiOjEwMDAwMDAsIkdhc1VzZWQiOjQ0NjI5fQ==",
        "Events": null,
        "Log": "",
        "Info": ""
      },
      "Key": null,
      "Value": null,
      "Proof": null,
      "Height": "0"
    }
  }
}
```

### Related Issue
- gnolang#1826

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: n3wbie <[email protected]>
Co-authored-by: Miloš Živković <[email protected]>
r3v4s added a commit to gnoswap-labs/gno that referenced this issue Dec 10, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->
## Descriptions

To simulate transactions, utilize the `.app/simulate` method for ABCI
Query.

### Changes
1. change the path of ABCI Query's `.app` to the result data storage
location.
- You can receive the query result data as
`RequestQuery.ResponseData.Data` instead of `RequestQuery.Value`.
- Provide it in a common form with other ABCI Queries.
2. remove the gas-consume logic of mocking signature data that is
executed when simulating transactions
([/tm2/pkg/sdk/auth/ante.go#L231-L237](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L231-L237))
- We will get the correct value when simulating a real transaction.
- We want transactions to run without signatures, but we already have
checks in place to see if a signature exists.
([tm2/pkg/sdk/auth/ante.go#L104-L106](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L104-L106))

### Example
#### [Request Simulate]
```curl
curl --location 'http://localhost:26657' \
--header 'Content-Type: application/json' \
--data '{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "abci_query",
  "params": [
    ".app/simulate",
    "CnMKDS9iYW5rLk1zZ1NlbmQSYgooZzFqZzhtdHV0dTlraGhmd2M0bnhtdWhjcGZ0ZjBwYWpkaGZ2c3FmNRIoZzFmZnp4aGE1N2RoMHFndjltYTV2MzkzdXIwemV4ZnZwNmxzanBhZRoMNTAwMDAwMHVnbm90Eg4IgIl6EggzMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohA+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2yEkCrIOTBt7YcDGcQ6Ohfv1r3nftAPaTATAtPfYD5zLQf7WDf1KPvWARe//CANtLLtIzcPVl7P/HnHxmfCYEwfGogIgUxMjMxMw==", 
    "0", 
    false
  ]
}'
```

#### [Response]
```curl
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "response": {
      "ResponseBase": {
        "Error": null,
        "Data": "eyJFcnJvciI6bnVsbCwiRGF0YSI6IiIsIkV2ZW50cyI6W10sIkxvZyI6Im1zZzowLHN1Y2Nlc3M6dHJ1ZSxsb2c6LGV2ZW50czpbXSIsIkluZm8iOiIiLCJHYXNXYW50ZWQiOjEwMDAwMDAsIkdhc1VzZWQiOjQ0NjI5fQ==",
        "Events": null,
        "Log": "",
        "Info": ""
      },
      "Key": null,
      "Value": null,
      "Proof": null,
      "Height": "0"
    }
  }
}
```

### Related Issue
- gnolang#1826

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: n3wbie <[email protected]>
Co-authored-by: Miloš Živković <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Onbloc (confirmed)
Status: In Progress
Status: In Progress
Development

No branches or pull requests

7 participants