-
Notifications
You must be signed in to change notification settings - Fork 229
drop proofs and headers received too late in the next round #7804
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
base: feat/testnet-fixes
Are you sure you want to change the base?
Changes from 2 commits
f5192bb
47e280e
ed3e604
d66e7bf
7dc66fd
5245d0c
f797214
147e242
a354c3d
1d0b2e2
03df0bc
03507d5
956cc86
9b9ec8a
e3a064b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||
| "sort" | ||||||||||||||||||||||||||||||||||||||||
| "sync" | ||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| "github.com/multiversx/mx-chain-core-go/core" | ||||||||||||||||||||||||||||||||||||||||
| "github.com/multiversx/mx-chain-core-go/core/check" | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,6 +26,9 @@ var log = logger.GetOrCreate("process/track") | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const maxNonceDifference = 3 // TODO move this to a config file | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // receivedProofDelay is the fraction of the full round time to accept a proof | ||||||||||||||||||||||||||||||||||||||||
| const receivedProofDelay = 0.5 | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // HeaderInfo holds the information about a header | ||||||||||||||||||||||||||||||||||||||||
| type HeaderInfo struct { | ||||||||||||||||||||||||||||||||||||||||
| Hash []byte | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -494,6 +498,18 @@ func (bbt *baseBlockTrack) checkAgainstRoundHandler(round uint64) error { | |||||||||||||||||||||||||||||||||||||||
| nextRound) | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| currentRoundStart := bbt.roundHandler.TimeStamp() | ||||||||||||||||||||||||||||||||||||||||
| roundDuration := float64(bbt.roundHandler.TimeDuration()) | ||||||||||||||||||||||||||||||||||||||||
| maxTimeToAcceptProof := time.Duration(roundDuration + roundDuration*receivedProofDelay) | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
it should be based on start round time, right?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, max time parameter is the total amount of time (900 ms in this case), as RemainingTime is called with currentRoundStart
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aa, right
|
||||||||||||||||||||||||||||||||||||||||
| roundDuration := float64(bbt.roundHandler.TimeDuration()) | |
| maxTimeToAcceptProof := time.Duration(roundDuration + roundDuration*receivedProofDelay) | |
| roundDuration := bbt.roundHandler.TimeDuration() | |
| extraWindow := time.Duration(float64(roundDuration.Nanoseconds()) * receivedProofDelay) | |
| maxTimeToAcceptProof := roundDuration + extraWindow |
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.
should be ok
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.
maybe use another error? this one might be misleading.
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.
updated
Copilot
AI
Mar 27, 2026
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.
The new time-window check is effectively inert with the real consensus/round implementation: RemainingTime(TimeStamp(), 1.5*TimeDuration) will stay >0 because TimeStamp() is updated each round and elapsed is always < TimeDuration(). As a result, headers/proofs won’t actually be dropped “when received too late in the next round”. Consider basing the window on the received item’s round (e.g., compute the start timestamp of that round and enforce acceptance only for the first receivedProofDelay fraction of it), or otherwise use a start time that doesn’t reset every round when validating next-round items.
| roundDuration := float64(bbt.roundHandler.TimeDuration()) | |
| maxTimeToAcceptProof := time.Duration(roundDuration + roundDuration*receivedProofDelay) | |
| timeLeftToAcceptProof := bbt.roundHandler.RemainingTime(currentRoundStart, maxTimeToAcceptProof) | |
| if timeLeftToAcceptProof <= 0 { | |
| return fmt.Errorf("%w header round: %d, current round timestamp: %d, time left to accept proof: %d", | |
| process.ErrHigherRoundInBlock, | |
| round, | |
| currentRoundStart.UnixMilli(), | |
| roundDuration := bbt.roundHandler.TimeDuration() | |
| maxTimeToAcceptProof := time.Duration(float64(roundDuration) * (1 + receivedProofDelay)) | |
| roundDiff := int64(round) - int64(bbt.roundHandler.Index()) | |
| itemRoundStart := currentRoundStart.Add(time.Duration(roundDiff) * roundDuration) | |
| timeLeftToAcceptProof := bbt.roundHandler.RemainingTime(itemRoundStart, maxTimeToAcceptProof) | |
| if timeLeftToAcceptProof <= 0 { | |
| return fmt.Errorf("%w header round: %d, item round timestamp: %d, time left to accept proof: %d", | |
| process.ErrHigherRoundInBlock, | |
| round, | |
| itemRoundStart.UnixMilli(), |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "fmt" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/multiversx/mx-chain-core-go/core" | ||
| "github.com/multiversx/mx-chain-core-go/core/check" | ||
|
|
@@ -2371,7 +2372,9 @@ func TestBaseBlockTrack_CheckBlockAgainstRoundHandlerShouldWork(t *testing.T) { | |
| currentRound := int64(50) | ||
| bbt.SetRoundHandler( | ||
| &mock.RoundHandlerMock{ | ||
| RoundIndex: currentRound, | ||
| RoundIndex: currentRound, | ||
| RoundTimeStamp: time.Now(), | ||
| RoundTimeDuration: time.Second, | ||
| }, | ||
| ) | ||
|
|
||
|
|
@@ -2383,6 +2386,39 @@ func TestBaseBlockTrack_CheckBlockAgainstRoundHandlerShouldWork(t *testing.T) { | |
| assert.Nil(t, err) | ||
| } | ||
|
|
||
| func TestBaseBlockTrack_CheckBlockAgainstRoundHandlerShouldFailOnInvalidWindow(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| bbt := track.NewBaseBlockTrack() | ||
| currentRound := int64(50) | ||
| roundDuration := time.Millisecond * 200 | ||
| bbt.SetRoundHandler( | ||
| &mock.RoundHandlerMock{ | ||
| RoundIndex: currentRound, | ||
| RoundTimeStamp: time.Now(), | ||
| RoundTimeDuration: roundDuration, | ||
| RemainingTimeCalled: func(startTime time.Time, maxTime time.Duration) time.Duration { | ||
| currentTime := time.Now() | ||
| elapsedTime := currentTime.Sub(startTime) | ||
| remainingTime := maxTime - elapsedTime | ||
|
|
||
| return remainingTime | ||
| }, | ||
|
Comment on lines
+2403
to
+2409
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we use real RemainingTime method from round handler?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe use real round handler, and call UpdateRound several times to set the desired index (with lower test values)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that should work too, tried to keep it simple |
||
| }, | ||
| ) | ||
|
|
||
| hdr := &block.Header{ | ||
| Round: uint64(currentRound + 1), // proper round but received too late | ||
| } | ||
|
|
||
| // wait until after half of the next round passed | ||
| timeToSleep := roundDuration + time.Duration(float64(roundDuration)*0.6) | ||
| time.Sleep(timeToSleep) | ||
|
Comment on lines
+2420
to
+2422
|
||
| err := bbt.CheckBlockAgainstRoundHandler(hdr) | ||
| require.ErrorIs(t, err, process.ErrHigherRoundInBlock) | ||
| require.Contains(t, err.Error(), "current round timestamp") | ||
| } | ||
|
|
||
| // ------- CheckBlockAgainstFinal | ||
|
|
||
| func TestBaseBlockTrack_CheckBlockAgainstFinalNilHeaderShouldErr(t *testing.T) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.