Add endpoint to get recent gas prices #4277

hanssv · 2024-02-26T10:12:23Z

Replaces #4191 - with a branch in this repo for easier collaboration.

There are a few things to consider here.

A minimum gas price is only relevant when chain is congested
When a micro-block is full isn't entirely obvious
Min gas fee can also be somewhat miner specific (miner min gas fee configuration).

This PR is supported by Æternity Foundation and Æternity Crypto Foundation

ThomasArts · 2024-02-26T11:33:12Z

apps/aehttp/src/aehttp_logic.erl

+        MinGasPrices ->
+            MinPriceN = fun(N) ->
+                            case lists:min(lists:sublist(MinGasPrices, N)) of
+                                undefined -> 0;


In OTP25 lists:min([]) throws an exception, but never undefined?
Oh, I see the hack now:
7> lists:min([undefined, 1,3]).
1
😃

Moreover, minimum should then possibly be the consensus defined minimum?

Returning 0 works for me as a way to distinguish

is there empty space in blocks (then SDK should use minimum gas price)

all blocks full, but transactions using minimum gas price (then SDK should increase the gas price a bit to ensure tx would be mined)

Otherwise I would ask for a flag indicating that.

ThomasArts

Not sure you should use 0 as minimum instead of consensus minimum, but that would not be a problem in practise

dincho · 2024-02-26T12:26:28Z

apps/aehttp/test/aehttp_integration_SUITE.erl

+        #{ <<"blocks">> := 120, <<"min_gas_price">> := MinGasPrice120 },
+        #{ <<"blocks">> := 480, <<"min_gas_price">> := MinGasPrice480 }
+    ]} = http_request(Host, get, "recent-gas-prices", []),
+    MinGasPrice = 1000000000,


Not very convincing test :)

Hmm, no... Didn't look at it, just updated it to pass 😅

dincho

Same concern as @ThomasArts about protocol minimums

dincho · 2024-02-26T12:30:42Z

but that would not be a problem in practise

Why not, if the client solely relies on this result and the last X blocks are empty? 🤔

hanssv · 2024-02-26T14:16:03Z

but that would not be a problem in practise

Why not, if the client solely relies on this result and the last X blocks are empty? 🤔

I think 0 would signal quite well that we don't have any data - the alternative would be protocol minimum, but there is no miner accepting that so that would be equally broken.

davidyuk

@hanssv In the latest version, I can't distinguish if blocks are full and I should use min gas price + 1 or if there is space in blocks and I can use 1e9 (default min gas price). If sdk would always do min gas price + 1 then multiple sdk instances can increase the gas price without reason.

davidyuk · 2024-02-27T16:37:33Z

Another issue:

I've build this branch, run it with configuration for sdk tests
recent-gas-prices shows "min_gas_price": 0 everywhere
changed min gas price in sdk to 1e9 + 1
executed some sdk tests
recent-gas-prices shows "min_gas_price": 1000000001 everywhere
I'd waited for a minute
changed min gas price in sdk to 1e9 + 2
executed some sdk tests
recent-gas-prices shows "min_gas_price": 1000000001 everywhere, but I expected to get 1000000002 in 1-minute because all transactions made have gas price 1e9 + 2

hanssv · 2024-02-27T21:00:16Z

@hanssv In the latest version, I can't distinguish if blocks are full and I should use min gas price + 1 or if there is space in blocks and I can use 1e9 (default min gas price). If sdk would always do min gas price + 1 then multiple sdk instances can increase the gas price without reason.

As Dincho explained somewhere - 1e9 is not a fixed value, it can be set by the miner, so wether there is space or not is actually not relevant. If there are Txs in an interval you'll get the min gas price found, otherwise 0.

hanssv · 2024-02-27T21:35:42Z

Another issue:

* I've build this branch, run it with configuration for sdk tests

* `recent-gas-prices` shows "min_gas_price": 0 everywhere

* changed min gas price in sdk to 1e9 + 1

* executed some sdk tests

* `recent-gas-prices` shows "min_gas_price": 1000000001 everywhere

* I'd waited for a minute

* changed min gas price in sdk to 1e9 + 2

* executed some sdk tests

* `recent-gas-prices` shows "min_gas_price": 1000000001 everywhere, but I expected to get 1000000002 in 1-minute because all transactions made have gas price 1e9 + 2

Microseconds and milliseconds were mixed-up, so you'd have to wait for 1000 minutes to see the effect 🙈

Fixed now.

ThomasArts

Missed that in review before, but indeed, times 1000

hanssv · 2024-03-06T07:59:25Z

So we decided on sticking to this approach (reporting minimum gas price that made it on chain in the last X, Y and Z minutes) - but we'll also add a congestions metric (basically how many percent of of the available gas that was used in the the given time).

I will also sort out the caching properly - probably by an actual caching lib 😅

ThomasArts · 2024-03-06T12:49:45Z

apps/aehttp/src/aehttp_logic.erl

+        Cache ->
+            case kache:get(Cache, Hash) of
+                {ok, _} ->
+                    lager:info("ZZZ: ~p in cache", [Hash]);


possibly real log message or debug message that the cache was hit?

apps/aehttp/src/aehttp_logic.erl

hanssv mentioned this pull request Feb 26, 2024

Add endpoint to get recent gas prices #4191

Closed

ThomasArts reviewed Feb 26, 2024

View reviewed changes

ThomasArts previously approved these changes Feb 26, 2024

View reviewed changes

dincho reviewed Feb 26, 2024

View reviewed changes

dincho previously approved these changes Feb 26, 2024

View reviewed changes

hanssv dismissed stale reviews from dincho and ThomasArts via 6f20ade February 26, 2024 13:55

hanssv marked this pull request as ready for review February 26, 2024 14:19

dincho previously approved these changes Feb 26, 2024

View reviewed changes

hanssv dismissed dincho’s stale review via ee2810b February 26, 2024 14:48

dincho previously approved these changes Feb 26, 2024

View reviewed changes

ThomasArts previously approved these changes Feb 26, 2024

View reviewed changes

davidyuk reviewed Feb 27, 2024

View reviewed changes

hanssv dismissed stale reviews from ThomasArts and dincho via fef8d99 February 27, 2024 21:34

ThomasArts previously approved these changes Feb 28, 2024

View reviewed changes

davidyuk and others added 8 commits March 6, 2024 08:59

Add endpoint to get recent gas prices

4a7f4c2

Improve implementation with some caching

2b26035

Don't return undefined

08ca621

Make dialyzer happier

c9476b9

Always return min gas price per generation

25d1ebe

Better use a fold

5699f97

Improved implementation now minutes from top instead

2ce46d7

Again, don't return undefined

c61265c

hanssv added 3 commits March 6, 2024 08:59

Tweak min gas in HTTP integration for better test

6fd632d

Fix milliseconds/microseconds screwup

e19d511

Add utilization value and fix cache with kache

44e3579

hanssv dismissed ThomasArts’s stale review via 44e3579 March 6, 2024 10:18

hanssv force-pushed the davidyuk-recent-gas-prices branch from fef8d99 to 44e3579 Compare March 6, 2024 10:18

Add to rebar.lock and deps

0f5c052

ThomasArts reviewed Mar 6, 2024

View reviewed changes

apps/aehttp/src/aehttp_logic.erl Outdated Show resolved Hide resolved

Remove left behind logging

000d40e

ThomasArts approved these changes Mar 6, 2024

View reviewed changes

dincho approved these changes Mar 7, 2024

View reviewed changes

mitchelli approved these changes Mar 7, 2024

View reviewed changes

hanssv merged commit abc961d into master Mar 7, 2024
38 checks passed

hanssv deleted the davidyuk-recent-gas-prices branch March 7, 2024 14:02

hanssv added kind/feature Issues or PRs related to a new feature area/api Issues or PRs related to the public API labels Mar 12, 2024

davidyuk mentioned this pull request Mar 19, 2024

Show minimum gas price in recent blocks and utilization aeternity/aescan#689

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add endpoint to get recent gas prices #4277

Add endpoint to get recent gas prices #4277

hanssv commented Feb 26, 2024

ThomasArts Feb 26, 2024 •

edited

ThomasArts Feb 26, 2024

davidyuk Feb 27, 2024 •

edited

ThomasArts left a comment

dincho Feb 26, 2024

hanssv Feb 26, 2024

dincho left a comment

dincho commented Feb 26, 2024

hanssv commented Feb 26, 2024

davidyuk left a comment

davidyuk commented Feb 27, 2024

hanssv commented Feb 27, 2024

hanssv commented Feb 27, 2024

ThomasArts left a comment

hanssv commented Mar 6, 2024

ThomasArts Mar 6, 2024

Add endpoint to get recent gas prices #4277

Add endpoint to get recent gas prices #4277

Conversation

hanssv commented Feb 26, 2024

ThomasArts Feb 26, 2024 • edited

Choose a reason for hiding this comment

ThomasArts Feb 26, 2024

Choose a reason for hiding this comment

davidyuk Feb 27, 2024 • edited

Choose a reason for hiding this comment

ThomasArts left a comment

Choose a reason for hiding this comment

dincho Feb 26, 2024

Choose a reason for hiding this comment

hanssv Feb 26, 2024

Choose a reason for hiding this comment

dincho left a comment

Choose a reason for hiding this comment

dincho commented Feb 26, 2024

hanssv commented Feb 26, 2024

davidyuk left a comment

Choose a reason for hiding this comment

davidyuk commented Feb 27, 2024

hanssv commented Feb 27, 2024

hanssv commented Feb 27, 2024

ThomasArts left a comment

Choose a reason for hiding this comment

hanssv commented Mar 6, 2024

ThomasArts Mar 6, 2024

Choose a reason for hiding this comment

ThomasArts Feb 26, 2024 •

edited

davidyuk Feb 27, 2024 •

edited