all : implement dynamicGasPrice(kip-71) #1493

kjeom · 2022-07-06T08:40:45Z

Proposed changes

This PR is merged from followings
This PR includes following features
- Governance parameters for KIP-71
- BaseFee dynamically updated from governance parameters and parent baseFee
- TxPool policy with gasPrice (baseFee)
- APIs for dynamicGasPrice

Types of changes

Please put an x in the boxes related to your change.

Bugfix
New feature or enhancement
Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have read the CONTRIBUTING GUIDELINES doc
I have signed the CLA
Lint and unit tests pass locally with my changes ($ make test)
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)
Any dependent changes have been merged and published in downstream modules

Related issues

Implemented dynamic gas fee pricing mechanism(KIP-71) #1245

Further comments

The baseFee increases/decreases only by up to 5% per block
The default gasTarget is 30 million that means at least about 1500 value transfer txs are needed to increase the baseFee
You can change governance parameters for kip71 by voting
If you want to set the kip71 hardforked block number in genesis.json, use homi option with kip71-compatible-blocknumber number
A baseFee field is added to the block header, so a hard fork is required.

…ist of transactions that greater than or equal to the given baseFee.

…ponding nonce already exists, a transaction with a high gasPrice is added.

…tx pool.

…of newTx is bigger than gasPrice of oldTx, replace it.

Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>

hqjang-pepper

Nice work :)

aidan-kwon

aidan-kwon

When a node processes a block, it doesn't verify the gas price of block transactions whether it is higher than the block base fee.

aidan-kwon · 2022-07-06T16:18:48Z

api/api_ethereum.go

@@ -1464,10 +1493,14 @@ func (api *EthereumAPI) rpcMarshalHeader(head *types.Header) (map[string]interfa
 		"receiptsRoot":     head.ReceiptHash,
 	}

+	// TODO-Klaytn needs to add kip71 hardfork condition


Do we need this line?

aidan-kwon · 2022-07-07T01:06:14Z

params/config.go

+	// TODO-klaytn temporal number for test
+	KIP71CompatibleBlockNum = big.NewInt(95000000)


Time to remove this

We can fix the block number now? I think we can fix this after the hardforked block number is confirmed.

jimni1222 · 2022-07-07T01:59:24Z

@kjeom Can you resolve conflict?

kjeom · 2022-07-07T02:06:08Z

@jimni1222 Yes, I resolved the conflict twice.

aeharvlee · 2022-07-07T02:32:47Z

@kjeom
[Issue Reporting]

CN: Cannot process the tx.

Logs

Full Logs
Below logs are appended continuously and KCN cannot process the txs.

images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:138]  [RC] Prepare messages received before startNewRound  address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" from=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd round=8069 len(prepares)=0 messages="\n"
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:140]  [RC] Commit messages received before startNewRound  address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" from=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd round=8069 len(commits)=0 messages="\n"
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:142]  [RC] Received 2f+1 Round Change Messages. Starting new round  address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" from=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd currentRound=8069 newRound=8069
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/preprepare.go:95]    Failed to verify proposal                 address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd from=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" err="invalid baseFee: have 25000000000, want 190020286334, parentBaseFee 200000000000, parentGasUsed 60859" duration=0s
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:42]   [RC] sendNextRoundChange happened         where="handlePreprepare. Proposal verification failure. Not ErrFutureBlock"
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:57]   [RC] Prepare messages received before catchUpRound  address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" len(prepares)=0 messages="\n"
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/roundchange.go:59]   [RC] Commit messages received before catchUpRound  address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" len(commits)=0 messages="\n"
images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/core.go:310]         [RC] Catch up round                       address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd old_round=8069 old_seq=195 old_proposer=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd new_round=8070 new_seq=195 new_proposer=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd

Problem Log

images-CN-0-1  | WARN[07/06,21:31:41 -04] [25|consensus/istanbul/core/preprepare.go:95]    Failed to verify proposal                 address=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd from=0x73718C4980728857f3Aa5148E9d1B471eFa3a7dd state="Accept request" err="invalid baseFee: have 25000000000, want 190020286334, parentBaseFee 200000000000, parentGasUsed 60859" duration=0s

This error is produced by

func VerifyKIP71Header(config *params.ChainConfig, parentHeader, header *types.Header) error {
	if header.BaseFee == nil {
		return fmt.Errorf("header is missing baseFee")
	}
	// Verify the baseFee is correct based on the parent header.
	expectedBaseFee := NextBlockBaseFee(parentHeader, config)
	if header.BaseFee.Cmp(expectedBaseFee) != 0 {
		return fmt.Errorf("invalid baseFee: have %s, want %s, parentBaseFee %s, parentGasUsed %d",
			header.BaseFee, expectedBaseFee, parentHeader.BaseFee, parentHeader.GasUsed)
	}
	return nil
}

Problem Situation

As you can see, there is just one tx for some blocks. Most of blocks contains ZERO transactions.

Block number	Base fee per gas	txs in a block
`183`	`0x5d21dba00`	`1`
`185`	`0x5d21dba00`	`1`
`187`	`0x5d21dba00`	`1`
`192`	`0x5d21dba00`	`1`
`193`	`0x5d21dba00`	`0`
`194`	`0x2e90edd000` (= 200 ston)	`1`

After block number 194, the error occurred.

How to Re-produce issue
If you have problem when re-produce the situation, feel free to contact me :)

git clone https://github.com/klaytn/caver-java.git
cd caver-java/.circleci/images
docker-compose up -d
docker-compose logs -f # Check the container status, you can see also KCN logs after KCN is up

After 5 mins later... (It takes few minutes to KCN up)
Run unit test

I recommended run this unit test using Intelli J Ultimate

cc. @jimni1222

kjhman21 · 2022-07-07T06:29:43Z

@kjeom @aidan-kwon Let's merge this and resolve the bug in other PRs.
cc @jimni1222 @aeharvlee @bobpkr

kale and others added 30 commits May 16, 2022 14:39

modify transaction validation in tx pool.

f799abb

add a ReadyWithGasPrice() which retrieves a sequentially increasing l…

2dc2eba

…ist of transactions that greater than or equal to the given baseFee.

apply ReadyWithGasPrice() and add test.

1f7c7b7

When adding tranasction to the list, if a transaction with the corres…

9c07800

…ponding nonce already exists, a transaction with a high gasPrice is added.

Implemented TxByTime sort and apply it.

a3b6242

Implemented FilterTransactionWithBaseFee() and apply it.

34fcef3

add TODO comment.

6f878a5

Demote transactions with gasPrice lower than the gasPrice set in the …

1bd2d46

…tx pool.

fix lint error

7fa74ba

modified test.

bbede0e

Modified txList in transaction replace logic correctly : If gasPrice …

732b641

…of newTx is bigger than gasPrice of oldTx, replace it.

modified test.

4c1940f

modify linter error.

c31763b

add initializing base fee governance parameters when booting node

8cfd960

will add BaseFee field in the other PR

e48d381

Apply suggestions from code review

1c2e94c

Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>

modified mistypo.

b53e7ac

Update blockchain/tx_list.go

adf06f1

Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>

fix lint error

d1d43e3

modified lint error

ef16c9a

modified lint error

699415a

modified linter

f29708b

changed kip71 config loading point

c905083

removed malformed comma (json) blockchain/genesis_alloc.go

45948c0

apply sorting TxByTime

3c32ae7

set KIP71 forked block number when creating node

ae56519

governance DB save test and minor test fix

0381f3e

add homi config for kip71 basefee parameters

8758b5c

Update params/config.go

44e8b8e

Co-authored-by: Junghyun Colin Kim <colin.klaytn@krustuniverse.com>

changed read locking to lock when setting blockchain field

5a2c748

kjeom requested review from jeongkyun-oh, jimni1222, sirano11, aeharvlee, JayChoi1736, nohkwak and 2dvorak as code owners July 6, 2022 08:40

kjeom assigned kjeom and ghost Jul 6, 2022

kjeom added the need to merge Need to merge for the next time label Jul 6, 2022

kjeom added this to the v1.9.0 milestone Jul 6, 2022

hqjang-pepper previously approved these changes Jul 6, 2022

View reviewed changes

Merge branch 'dev' into dynamicGasPrice

d22cbe9

kjeom dismissed hqjang-pepper’s stale review via d22cbe9 July 6, 2022 15:11

aidan-kwon reviewed Jul 6, 2022

View reviewed changes

remove comments

e127f56

aidan-kwon reviewed Jul 7, 2022

View reviewed changes

Merge branch 'dev' into dynamicGasPrice

420c609

aidan-kwon approved these changes Jul 7, 2022

View reviewed changes

jimni1222 approved these changes Jul 7, 2022

View reviewed changes

hqjang-pepper approved these changes Jul 7, 2022

View reviewed changes

yoomee1313 approved these changes Jul 7, 2022

View reviewed changes

jeongkyun-oh approved these changes Jul 7, 2022

View reviewed changes

kjeom merged commit eb48bd7 into dev Jul 7, 2022

blukat29 removed the need to merge Need to merge for the next time label Feb 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all : implement dynamicGasPrice(kip-71) #1493

all : implement dynamicGasPrice(kip-71) #1493

kjeom commented Jul 6, 2022 •

edited

hqjang-pepper left a comment

aidan-kwon left a comment •

edited

aidan-kwon left a comment

aidan-kwon Jul 6, 2022

aidan-kwon Jul 7, 2022

kjeom Jul 7, 2022

jimni1222 commented Jul 7, 2022

kjeom commented Jul 7, 2022

aeharvlee commented Jul 7, 2022 •

edited

kjhman21 commented Jul 7, 2022

		// TODO-klaytn temporal number for test
		KIP71CompatibleBlockNum = big.NewInt(95000000)

all : implement dynamicGasPrice(kip-71) #1493

all : implement dynamicGasPrice(kip-71) #1493

Conversation

kjeom commented Jul 6, 2022 • edited

Proposed changes

Types of changes

Checklist

Related issues

Further comments

hqjang-pepper left a comment

Choose a reason for hiding this comment

aidan-kwon left a comment • edited

Choose a reason for hiding this comment

aidan-kwon left a comment

Choose a reason for hiding this comment

aidan-kwon Jul 6, 2022

Choose a reason for hiding this comment

aidan-kwon Jul 7, 2022

Choose a reason for hiding this comment

kjeom Jul 7, 2022

Choose a reason for hiding this comment

jimni1222 commented Jul 7, 2022

kjeom commented Jul 7, 2022

aeharvlee commented Jul 7, 2022 • edited

kjhman21 commented Jul 7, 2022

kjeom commented Jul 6, 2022 •

edited

aidan-kwon left a comment •

edited

aeharvlee commented Jul 7, 2022 •

edited