-
Notifications
You must be signed in to change notification settings - Fork 52
refactor: support optional cardano transaction signing config #2789
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
Open
Alenar
wants to merge
4
commits into
main
Choose a base branch
from
djo/2780/support-optional-ctx-config
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+356
−244
Conversation
This file contains hidden or 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
this simplify a bit the management and usage of its stored values
…ochSettings` Note: this commit compiles, but break aggregator tests as init of test data in db will fails because of missing config when inserting to `epoch_setting` table.
…base Note: this commit compiles, but aggregator tests are still failing because `EpochService::inform_epoch` still need a ctx signing config.
…ignedEntityConfig` This make the network resilient to a removal of this configuration at the expense of moving the error handling to nodes 'beacon to sign' computations (done in their state machines). Notes: leader aggregator will fails directly at startup if an incoherent configuration, which enable cardano transactions but don't specifies the associated configuration, is provided.
jpraynaud
approved these changes
Nov 19, 2025
Member
jpraynaud
left a comment
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 👍
| SqlMigration::new( | ||
| 37, | ||
| r#" | ||
| -- disable foreign keys since `signer_registration` have a foreign key constraint on `epoch_setting` |
Member
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.
Suggested change
| -- disable foreign keys since `signer_registration` have a foreign key constraint on `epoch_setting` | |
| -- disable foreign keys since `signer_registration` has a foreign key constraint on `epoch_setting` |
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.
Content
This PR makes optional the cardano transaction signing configuration everywhere.
This move the error raised when this configuration is missing from the nodes startup (for the aggregator), or when deriving any time point to a signed entity (in the signer state machine), to deriving the specific derivation of a time point to the
CardanoTransactionssigned entity.This will allow to remove this parameter, if needed in the future, without making any nodes fails without any ways to recover.
Details
Mithril common
SignedEntityConfig:cardano_transactions_signing_configoptionaltime_point_to_signed_entitywhen the asked discriminant isCardanoTransactionsand thecardano_transactions_signing_configis NoneMithril protocol config
HttpMithrilNetworkConfigurationProvider::get_network_configurationandconfiguration_for_aggregation.signed_entity_types_config.cardano_transactionsis None.Mithril aggregator
MithrilNetworkConfigurationdirectly in anetwork_configurationmethod, the three protocol parameters methods are now shortcut to thenetwork_configurationcardano_transactions_signing_configfield is now optional inAggregatorEpochSettingscardano_transactions_signing_configfield is now optional inEpochSettingsRecordcardano_transactions_signing_configfield nullable in theepoch_settingtableSignedEntityConfigchangesMithril signer
SignedEntityConfigchangesPre-submit checklist
Issue(s)
Closes #2780