[Enhancement] Add Gas Price Oracle User Configuration #4362

Aishlia · 2023-02-10T23:11:26Z

Issue

#4357 Add feature to enable users to specify gas price parameters (blocks, percentile, defaultPriceGwei, maxPriceGwei, and numberTxsnSampled) in config and as flags.

Test

Unit Test Coverage

Before:

ok      github.com/harmony-one/harmony/cmd/harmony    0.756s    coverage: 44.8% of statements

After:

PASS
ok  	github.com/harmony-one/harmony/cmd/harmony	0.568s

Test/Run Logs

julianai@Harmonys-MacBook-Air harmony % go test ./cmd/harmony
# gopkg.in/olebedev/go-duktape.v3
In file included from _cgo_export.c:4:
debugger.go:23:13: warning: unused function '_duk_debugger_attach' [-Wunused-function]
ok  	github.com/harmony-one/harmony/cmd/harmony	0.495s
julianai@Harmonys-MacBook-Air harmony % go test -run TestHarmonyFlags
# gopkg.in/olebedev/go-duktape.v3
In file included from _cgo_export.c:4:
debugger.go:23:13: warning: unused function '_duk_debugger_attach' [-Wunused-function]
PASS
ok  	github.com/harmony-one/harmony/cmd/harmony	0.497s

Operational Checklist

Does this PR introduce backward-incompatible changes to the on-disk data structure and/or the over-the-wire protocol?. (If no, skip to question 8.)

NO
Describe the migration plan.. For each flag epoch, describe what changes take place at the flag epoch, the anticipated interactions between upgraded/non-upgraded nodes, and any special operational considerations for the migration.
Describe how the plan was tested.
How much minimum baking period after the last flag epoch should we allow on Pangaea before promotion onto mainnet?
What are the planned flag epoch numbers and their ETAs on Pangaea?
What are the planned flag epoch numbers and their ETAs on mainnet?

Note that this must be enough to cover baking period on Pangaea.
What should node operators know about this planned change?
Does this PR introduce backward-incompatible changes NOT related to on-disk data structure and/or over-the-wire protocol? (If no, continue to question 11.)

NO
Does the existing node.sh continue to work with this change?
What should node operators know about this change?
Does this PR introduce significant changes to the operational requirements of the node software, such as >20% increase in CPU, memory, and/or disk usage?

NO

TODO

Mainnet Release Candidate 4.3.14-rc3 (dev -> main)

MaxMustermann2

Please update test TestHarmonyFlags as well.

rosetta/infra/harmony-pstn.conf

cmd/harmony/main.go

cmd/harmony/flags.go

rosetta/infra/harmony-mainnet.conf

MaxMustermann2 · 2023-02-14T18:08:00Z

cmd/harmony/flags.go

+	}
+	gasPriceOracleMaxPriceGweiFlag = cli.IntFlag{
+		Name:     "gpo.maxprice",
+		Usage:    "Maximum possible gas price in gwei returned by eth_gasPrice (default: 12)",


Why is max gas price (12) < default gas price (100) ?

MaxMustermann2 · 2023-02-14T18:08:49Z

cmd/harmony/flags_test.go

+					Percentile:       80,
+					DefaultPriceGwei: 100,
+					MaxPriceGwei:     12,
+					NumberTxsSampled: 3,


Refer to the values here instead of hard coding them.

MaxMustermann2 · 2023-02-14T18:11:12Z

cmd/harmony/main.go

@@ -803,6 +803,9 @@ func setupConsensusAndNode(hc harmonyconfig.HarmonyConfig, nodeConfig *nodeconfi
 		Uint64("viewID", viewID).
 		Msg("Init Blockchain")

+	// Assign GasPriceOracle parameters to the current node
+	currentNode.HarmonyConfig.GasPriceOracle = hc.GasPriceOracle


I still don't see why this is needed. The currentNode stores within itself the entire config (including the GasPriceOracle) per below.

harmony/node/node.go

Line 1043 in b2e50e6

node.HarmonyConfig = harmonyconfig

MaxMustermann2 · 2023-02-14T18:11:47Z

go.mod

@@ -71,6 +71,7 @@ require (
 	github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b
 	github.com/ledgerwatch/erigon-lib v0.0.0-20221218022306-0f8fdd40c2db
 	github.com/ledgerwatch/log/v3 v3.6.0
+	github.com/libp2p/go-libp2p-core v0.20.1
 )


It's preferred not to commit go.mod and go.sum for minor changes.

MaxMustermann2 · 2023-02-14T18:12:42Z

hmy/gasprice.go

+					"updated",
+					DefaultGasPriceOracleConfig.NumberTxsSampled,
+				),
+			)
 	}


Add a sanity check for maxPrice < defaultPrice

MaxMustermann2 · 2023-02-14T18:13:30Z

hmy/staking.go

@@ -387,7 +387,7 @@ func (hmy *Harmony) GetValidatorInformation(
 	// b.apiCache.Forget(prevKey)

 	// calculate last APRHistoryLength epochs for averaging APR
-	// epochFrom := bc.GasPriceConfig().StakingEpoch
+	// epochFrom := bc.GasPriceOracleConfig().StakingEpoch


Undo this change.

MaxMustermann2 · 2023-02-14T18:13:48Z

hmy/hmy.go

-		Default:    big.NewInt(100e9), // minimum of 100 gwei
-	}
+	gpoParams := nodeAPI.GetHarmonyConfig().GasPriceOracle
+


nit: Remove this blank line.

ONECasey · 2023-02-15T17:22:34Z

A few merge conflicts after a rebase.

ONECasey · 2023-02-17T01:57:18Z

Fixed the merge conflicts for you.

fix fix fix fix

If a contract self destructs to self and then receives funds within the same transaction, it is possible for its stale state to be saved. This change removes that possibility by checking for deleted state objects before returning them.

ONECasey · 2023-03-14T23:42:24Z

New dev. Needs mentorship. Closed

This change makes the gas price oracle options available as options in the configuration file / command line. In addition, the gas price oracle's suggestion mechanism has been modified to return the default gas price when block utilization is low. In other words, the oracle can be configured to return the 60th percentile gas price from the last 5 blocks with 3 transactions each, or return the default gas price if those 5 blocks were utilized less than 50% of their capacity Fixes harmony-one#4357 and supersedes harmony-one#4362

This change makes the gas price oracle options available as options in the configuration file / command line. In addition, the gas price oracle's suggestion mechanism has been modified to return the default gas price when block utilization is low. In other words, the oracle can be configured to return the 60th percentile gas price from the last 5 blocks with 3 transactions each, or return the default gas price if those 5 blocks were utilized less than 50% of their capacity Fixes #4357 and supersedes #4362

sophoah and others added 3 commits January 23, 2023 14:24

Merge pull request harmony-one#4342 from harmony-one/dev

09dba41

Mainnet Release Candidate 4.3.14-rc3 (dev -> main)

Dockerfile update apk add software version

e3cabb8

github action - update

b2e50e6

Aishlia requested review from Frozen, MaxMustermann2, angelzhangjs and ONECasey February 10, 2023 23:11

Aishlia self-assigned this Feb 10, 2023

Aishlia closed this Feb 10, 2023

Aishlia reopened this Feb 10, 2023

Aishlia closed this Feb 11, 2023

Aishlia reopened this Feb 11, 2023

MaxMustermann2 suggested changes Feb 11, 2023

View reviewed changes

rosetta/infra/harmony-pstn.conf Show resolved Hide resolved

cmd/harmony/main.go Outdated Show resolved Hide resolved

cmd/harmony/flags.go Outdated Show resolved Hide resolved

ONECasey reviewed Feb 11, 2023

View reviewed changes

rosetta/infra/harmony-mainnet.conf Show resolved Hide resolved

MaxMustermann2 suggested changes Feb 14, 2023

View reviewed changes

ONECasey requested review from ONECasey and MaxMustermann2 February 15, 2023 17:22

Frozen added 11 commits March 2, 2023 20:02

Small fixes.

e9129bd

in progress.

c7c26c5

in progress.

2f4a508

in progress.

76b8ef6

consensus check is forked

ace5369

update master

543b6ad

fix leader

2e5f3b2

check leader for N blocks

4f28ac2

fix

a6655f0

fix

048e03b

Cleanup and fix update pub keys.

c95bbc0

Frozen and others added 24 commits March 2, 2023 20:10

Cleanup and fix update pub keys.

b73d8a1

Rotate leader.

7ad6a98

fix

6499338

fix fix fix fix

Cleaned.

5d3a160

Cache for GetLeaderPubKeyFromCoinbase, removed NthNextHmyExt.

141880a

comment activation

2e151e9

295 epoch

e7ce5ce

Fix failed tests.

024d7da

Fixed code review.

0e18c0b

Fix review comments.

667cba2

Merged leader rotation.

27d20ab

Rebased on dev.

7abd34f

Rebased on dev.

a52e5f7

Fix usage of private methods.

b6e6e13

Fix usage of private methods.

cf42754

Fix usage of private methods.

04998a6

Removed deadcode, LockedFBFTPhase.

8b9fcc2

Fix review comment.

a3de6b8

Fix review comment.

67ba9e1

Set to EpochTBD.

128433f

Fix tests.

8f96cba

[core] fix state handling of self destruct

a3131ba

If a contract self destructs to self and then receives funds within the same transaction, it is possible for its stale state to be saved. This change removes that possibility by checking for deleted state objects before returning them.

Changed default MaxPriceGwei to 1000

688ce01

Changed default MaxPriceGwei to 1000

7c18d6f

Aishlia force-pushed the dev branch from 0597340 to 7c18d6f Compare March 7, 2023 02:05

ONECasey closed this Mar 14, 2023

MaxMustermann2 mentioned this pull request May 22, 2023

hmy: Gas Price Oracle improvements #4436

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] Add Gas Price Oracle User Configuration #4362

[Enhancement] Add Gas Price Oracle User Configuration #4362

Aishlia commented Feb 10, 2023 •

edited

Loading

MaxMustermann2 left a comment

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

MaxMustermann2 Feb 14, 2023

ONECasey commented Feb 15, 2023

ONECasey commented Feb 17, 2023

ONECasey commented Mar 14, 2023

[Enhancement] Add Gas Price Oracle User Configuration #4362

[Enhancement] Add Gas Price Oracle User Configuration #4362

Conversation

Aishlia commented Feb 10, 2023 • edited Loading

Issue

Test

Unit Test Coverage

Test/Run Logs

Operational Checklist

TODO

MaxMustermann2 left a comment

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

MaxMustermann2 Feb 14, 2023

Choose a reason for hiding this comment

ONECasey commented Feb 15, 2023

ONECasey commented Feb 17, 2023

ONECasey commented Mar 14, 2023

Aishlia commented Feb 10, 2023 •

edited

Loading