consensus: annotate headers with their slot's RelativeTime #1288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amesgen
reviewed
Oct 23, 2024
...oros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/NodeKernel.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-diffusion/test/consensus-test/Test/Consensus/PeerSimulator/Run.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Genesis/Governor.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Genesis/Governor.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Genesis/Governor.hs
Outdated
Show resolved
Hide resolved
...ensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/BlockFetch/ClientInterface.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HeaderValidation.hs
Outdated
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/test/consensus-test/Test/Consensus/MiniProtocol/BlockFetch/Client.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/test/consensus-test/Test/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
5 tasks
dnadales
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
b76d77c to
2b11f09
Compare
amesgen
reviewed
Nov 21, 2024
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/HeaderValidation.hs
Outdated
Show resolved
Hide resolved
...ensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/BlockFetch/ClientInterface.hs
Outdated
Show resolved
Hide resolved
...boros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Util/AnchoredFragment.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/unstable-consensus-testlib/Test/Util/HeaderValidation.hs
Outdated
Show resolved
Hide resolved
dnadales
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
1b5a867 to
6f9c71b
Compare
Closed
7 tasks
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
2 times, most recently
from
7296216 to
f40c927
Compare
nfrisby
reviewed
Dec 12, 2024
...nsus/src/ouroboros-consensus/Ouroboros/Consensus/HardFork/Combinator/Abstract/NoHardForks.hs
Outdated
Show resolved
Hide resolved
|
I pushed up some commits that improve some names&Haddock and also hide |
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
ccc97d4 to
566d1e5
Compare
|
I changed the representation of the fragment pair based on the conversation we had in today's office hours. I confirmed that the statemachine test fails (nicely) when I insert bugs in either ChainSel or Background's maintenance of the fragment with times. I introduced a bespoke strict pair type for clarity and to most easily satisfy the NoThunks tests (which are also enabled by the Cabal flags that enable the new invariant checking logic for the fragment pair). |
nfrisby
reviewed
Dec 13, 2024
| @@ -339,6 +341,39 @@ initNodeKernel args@NodeKernelArgs { registry, cfg, tracers | |||
| blockForging' <- traverse (forkBlockForging st) blockForging | |||
| go blockForging' | |||
|
|
|||
| castTraceFetchClientState :: | |||
There was a problem hiding this comment.
I opened IntersectMBO/ouroboros-network#5025 to upstream this function
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
5 times, most recently
from
c625852 to
4cb2c5e
Compare
|
I squashed in order to rebase. I also minized the diff. The next step is to split it into two or three separate commits. |
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
4 times, most recently
from
ff91b8a to
0ffd370
Compare
This PR will add ImmutableEraParams instances for test blocks without needing the SingleEraBlock omnibus.
GHC 9.4+ "erroneously" requires HasHeader blk. This patch prevents that. I _think_ the new local signature forces GHC to invoke the HeaderHash (Header blk) ~ HeaderHash blk rewrite _later_, thereby avoiding that undesirable Wanted constraint.
It's declared upstream in Control.ResourceRegistry now.
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
0ffd370 to
2effa46
Compare
- Define HeaderWithTime. - Maintain time annotations on the ChainSync candidates. - Maintain a parallel selection with time annotations since the ConsensusBlockFetch interface uses the same type argument for ChainSync candidates and the current selection. - Note that HeaderWithTime is hidden from the tracer events, at least for now. Co-authored-by: Damian Nadales <[email protected]> Co-authored-by: amesgen <[email protected]>
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
2effa46 to
ee2f301
Compare
nfrisby
reviewed
Dec 18, 2024
|
|
||
| instance ( HasHeader (Header blk) | ||
| , StandardHash (HeaderWithTime blk) | ||
| , Typeable blk |
There was a problem hiding this comment.
This is the root cause of all the additional Typeable blk contexts in this commit. It's required to satisfy the superclass on the (upstream) HasHeader. I don't see how to avoid it, sadly.
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
ee2f301 to
ba54272
Compare
nfrisby
force-pushed
the
dnadales/annotate-headers-with-time
branch
from
ba54272 to
b4424eb
Compare
nfrisby
changed the title
nfrisby
requested review from
jasagredo,
fraser-iohk and
geo2a
as code owners
|
UTxO HD takes priority over this, if the conflicts are non-trivial. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a new
HeaderWithTimedata type, which annotates each received header with the relative time of its slot as calculated by the ChainSync client.Partially addresses #1301