-
Notifications
You must be signed in to change notification settings - Fork 41
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
Refactor timepoint retrieval & conversion to signed entities #1736
Conversation
Test Results 3 files ±0 43 suites ±0 8m 36s ⏱️ -1s Results for commit 179f66d. ± Comparison against base commit 2ebb8c4. This pull request removes 13 and adds 19 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
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
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 👍
I left few comments below.
Note: there's still a conflicting `TickerService` defined in the aggregator, this will be solved in a later commit. Co-authored-by: Sébastien Fauvel <[email protected]>
and remove it's own version of the service. Co-authored-by: Sébastien Fauvel <[email protected]>
A shortcut method for the cases were only the epoch is needed Co-authored-by: Sébastien Fauvel <[email protected]>
To `ticker_service`. Co-authored-by: Sébastien Fauvel <[email protected]>
This static configuration contains all what's needed to convert a `TimePoint` to a `SignedEntityType` (given a discriminant). It's hold by the TickerService and copied into each created time point so they don't need anything more for conversions (no additional services needed at a price of a more complex struct). Also add `SignedEntityTypeDiscriminants::all` to list all available discriminent using `StrumIterm` (so they're always updated). Co-authored-by: Sébastien Fauvel <[email protected]> Co-authored-by: Damien Lachaume <[email protected]>
For signers, even if they have no use right now, they are configured as: - allowed discriminants: all - cardano_transactions_signing_config: security parameter '3000' and step '120' (same as aggregator).
…ormer Since the `TimePoint` now hold everything needed for that conversion.
Copied & adapted from aggregator `Configuration`. Co-authored-by: Damien Lachaume <[email protected]>
Instead of putting everything in the `TimePoint`, this made the timepoint cumbersome to use in the numerous cases when we did not needs those functionnalities. Co-authored-by: Damien Lachaume <[email protected]>
Co-authored-by: Damien Lachaume <[email protected]>
As this responsability is now handled by the `SignedEntityConfig`. Co-authored-by: Damien Lachaume <[email protected]>
* Remove now unused `network` field from `AggregatorConfig`. * Make builder `get_signed_entity_config` pub like its other 'get' methods. * fix the default `CardanoImmutableFilesFull` signed entity construction in the `/register-signatures` root handler. Co-authored-by: Damien Lachaume <[email protected]>
From the aggregator configuration. Co-authored-by: Sébastien Fauvel <[email protected]> Co-authored-by: Damien Lachaume <[email protected]>
Before the were simply ignored Co-authored-by: Sébastien Fauvel <[email protected]>
1087a20
to
bd0311e
Compare
Clarify some documentations and naming. Co-authored-by: Sébastien Fauvel <[email protected]>
* Mithril-aggregator from `0.5.17` to `0.5.18` * Mithril-signer from `0.2.141` to `0.2.142` * Mithril-common from `0.4.13` to `0.4.14`
bd0311e
to
179f66d
Compare
Content
Following changes introduced in #1727 the conversion of time points to signed entities became more cumbersome. Instead of only using a
CardanoNetwork
it also needed now the newly introducedCardanoTransactionsSigningConfig
.This PR make this easier by delegating the configuration to a new in memory type:
SignedEntityConfig
, that is created at configuration time and is in the application dependency container.This type also takes responsibility, previously in the aggregator
Configuration
, to list allowed signed entity types from a time point. It also hold statically the list of default types that should always be allowed.Also in this PR: Rename
TimePointProvider
toTickerService
and remove the conflicting service, named the same, from the aggregator.Those two were doing the same things and comments were in the code to remove the
TimePointProvider
in favor of theTickerService
, but the former continued to evolved even if the later api & naming were preferred. So this PR fixes this contradiction by "fusing" them together.Pre-submit checklist
Issue(s)
Relates to #1697 and #1727