-
Notifications
You must be signed in to change notification settings - Fork 324
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
fix test and speedup validation #3842
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3842 +/- ##
=======================================
Coverage 75.38% 75.38%
=======================================
Files 303 303
Lines 25923 26014 +91
=======================================
+ Hits 19541 19610 +69
- Misses 5360 5379 +19
- Partials 1022 1025 +3
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
189d08d
to
2b01c59
Compare
@@ -15,5 +15,5 @@ import ( | |||
func TestTimestamp(t *testing.T) { | |||
assert := assert.New(t) | |||
time1 := TimestampNow() | |||
assert.True(time.Now().After(time1)) | |||
assert.False(time.Now().Before(time1)) |
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.
sometimes it happens that time.Now() == time1
, causing CI test to fail
@@ -110,7 +110,7 @@ type ( | |||
CorrectGasRefund bool | |||
FixRewardErroCheckPosition bool | |||
EnableWeb3Rewarding bool | |||
EnableNodeInfo bool | |||
SkipSystemActionNonce bool |
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.
EnableNodeInfo
is not used at all
} else { | ||
if err := ws.validateNonce(ctx, blk); err != nil { | ||
return errors.Wrap(err, "failed to validate nonce") | ||
} |
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.
block 1 has 2 tx, from the same sender io1vtm2zgn830pn6auc2cvnchgwdaefa9gr4z0s86
, both with nonce=0
, one is a transfer, the other is grant reward (system action)
validateNonce2()
will ignore the grant reward, but return error for the transfer tx (since it's expecting nonce=1 but got nonce=0), so we have to keep the height check and 2 validate()
state/factory/workingset.go
Outdated
|
||
// Special handling for genesis block | ||
if blk.Height() == 0 { | ||
return nil |
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.
duplicated with L338-L340, moved into checkNonceContinuity()?
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.
tested it's actually not needed
state/factory/workingset.go
Outdated
return ws.checkNonceContinuity(ctx, accountNonceMap) | ||
} | ||
|
||
func (ws *workingSet) validateNonce2(ctx context.Context, blk *block.Block) error { |
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.
better name than validateNonce2
?
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.
fixed
Kudos, SonarCloud Quality Gate passed! |
Description
backport haixiang's fix to master branch
Fixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: