-
Notifications
You must be signed in to change notification settings - Fork 113
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
Update TSC to compare tangle time with time of confirmed message. #2167
Conversation
packages/tangle/tipmanager.go
Outdated
@@ -363,8 +363,7 @@ func (t *TipManager) Tips(p payload.Payload, countParents int) (parents MessageI | |||
} | |||
|
|||
func (t *TipManager) isPastConeTimestampCorrect(messageID MessageID) (timestampValid bool) { | |||
now := clock.SyncedTime() | |||
minSupportedTimestamp := now.Add(-t.tangle.Options.TimeSinceConfirmationThreshold) | |||
minSupportedTimestamp := t.tangle.TimeManager.Time().Add(-t.tangle.Options.TimeSinceConfirmationThreshold) |
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.
With the new time types (CTT, RCTT, FTT, RFTT) added to the TimeManager, you might want to change that condition.
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.
Can we merge #2193 into this one before merging?
// if closest past marker is confirmed and before minSupportedTimestamp, then message should be ok | ||
return true | ||
// if closest past marker is confirmed and before minSupportedTimestamp, then need to walk message past cone of the previously marker message | ||
messageWalker.Push(previousMessageID) |
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.
Needed!
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.
wdym?
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.
We needed to push the message to the messageWalker to evaluate it; before we were just assuming that if the closest marker was confirmed the message was ok.
// if closest past marker is confirmed and before minSupportedTimestamp, then message should be ok | ||
return true | ||
// if closest past marker is confirmed and before minSupportedTimestamp, then need to walk message past cone of the previously marker message | ||
messageWalker.Push(previousMessageID) |
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.
We needed to push the message to the messageWalker to evaluate it; before we were just assuming that if the closest marker was confirmed the message was ok.
* Add CTT, FTT. Rename Synced to Bootstrapped * Add RCTT and RFTT to time manager * Change sync logic to bootstrap * Add new time types to the API responses * Use RCTT for activity window * Review fixes * Review fixes * Use to distinct states for bootstrapped and synced * Add bootstrapped check * Change RCTT calculation * Issue metrics only when node is synced. Co-authored-by: Daria Dziubałtowska <daria.dziubaltowska@iota.org> Co-authored-by: Piotr Macek <piotr.macek@iota.org>
Co-authored-by: Jonas Theis <4181434+jonastheis@users.noreply.github.com>
Description of change
Solves #2160
Type of change
Choose a type of change, and delete any options that are not relevant.
How the change has been tested
Describe the tests that you ran to verify your changes.
Make sure to provide instructions for the maintainer as well as any relevant configurations.
Unit tests, manual testing.
Change checklist
Add an
x
to the boxes that are relevant to your changes, and delete any items that are not.