-
Notifications
You must be signed in to change notification settings - Fork 182
Conversation
Co-authored-by: Jamie <32922423+jimni1222@users.noreply.github.com>
Co-authored-by: bob.p <106128487+bobpkr@users.noreply.github.com>
Co-authored-by: bob.p <106128487+bobpkr@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good, but we need to change the calculation logic in KIP71
return baseFee | ||
} | ||
|
||
func makeEvenByUp(baseFee *big.Int) *big.Int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kjeom hmm.. name is little bit misleading.
Can we use the name 'ceil' and 'floor' instead?
@@ -23,10 +23,27 @@ func VerifyMagmaHeader(config *params.ChainConfig, parentHeader, header *types.H | |||
return nil | |||
} | |||
|
|||
func makeEvenByDown(baseFee *big.Int) *big.Int { | |||
if baseFee.Bit(0) != 0 { | |||
baseFee.Rsh(baseFee, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to use baseFee.sub()
Refine Implementation of klaytn#1537
Refined this code please take a look @kjeom @jimni1222 @bobpkr @aidan-kwon |
Refine Implementation of klaytn#1537
Proposed changes
Types of changes
Please put an x in the boxes related to your change.
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.
$ make test
)