-
Notifications
You must be signed in to change notification settings - Fork 88
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 offline-mode #1222
Refactor offline-mode #1222
Conversation
079ccf5
to
5f2b7cf
Compare
Transactions CostsSizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using
Script summary
Cost of Init Transaction
Cost of Commit TransactionThis is using ada-only outputs for better comparability.
Cost of CollectCom Transaction
Cost of Close Transaction
Cost of Contest Transaction
Cost of Abort TransactionSome variation because of random mixture of still initial and already committed outputs.
Cost of FanOut TransactionInvolves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.
End-To-End Benchmark ResultsThis page is intended to collect the latest end-to-end benchmarks results produced by Hydra's Continuous Integration system from the latest Please take those results with a grain of salt as they are currently produced from very limited cloud VMs and not controlled hardware. Instead of focusing on the absolute results, the emphasis should be on relative results, eg. how the timings for a scenario evolve as the code changes. Generated at 2023-12-22 17:55:23.934216196 UTC Baseline Scenario
Baseline Scenario
|
1f86e4f
to
e680ad0
Compare
e680ad0
to
580ecdc
Compare
Test Results388 tests +7 383 ✅ +7 21m 18s ⏱️ +23s Results for commit 00427a5. ± Comparison against base commit b0c94e7. This pull request removes 3 and adds 10 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
0d6ab99
to
2aecc74
Compare
2aecc74
to
aa39f37
Compare
@cardenaso11 @rrruko @Quantumplation I can't request a review from you (I think I could, if I push this on your fork .. if you want?), but I would love your input on the clean up I did here. |
@ch1bo sure thing! |
aa39f37
to
2ecdd29
Compare
This is to ensure the refactor to be not breaking.
This was leading to a lot of duplication in the Hydra.Node.Run and HydraNode modules. Instead we should be able to switch quite easily between multiple `ChainConfig` variants.
This makes re-use of the `runOptionsParser` possible for "offline mode". Also it simplifies the tests, of which two were even broken hidden by the fact that a missing --node-id was the reason of a failing parse.
This emulates a sub-command, although the offline-mode would also just be available directly on the command-line without sub-commands.
Also moved the ChainConfig schema away from the api schema as it was not referenced there.
By extending the arbitrary instances, tests were checking that the toArgs matches what can be parsed.
This does not change the command line interface, but makes end-to-end tests of the offline chain not need to come up with this parameter.
This is now implemented using the cardano-api:internal lib and using those defaults. In general it is questionable if those 'globals' are actually used by the cardano-ledger we embed (as we only use a subset of the ledger rules).
We should never use record fields as functions.
Both, 'run' and 'initEnvironment' need to switch on the configured chain layer to do the plumbing correctly. Especially in the 'run' function this avoids a lot of duplication.
This allows (and forces) us to be more specific in type signatures of the two chain layers and other locations.
offline mode e2e test
This was not entirely clear from the previous test and ensures we do not break anything when simplifying the implementation.
This handle is not needed if we simulate an opening head in initializeStateIfOffline.
This also removes redundant parameters about head id, head seed and contestation period. All of them are irrelevant in offline mode and can be hard-coded.
This is only needed when we are "opening" the head the first time
This is to ensure it works correctly before refactoring anything.
This now loads the given genesis file / uses the defaults just the same as before, but does not require the additional 'globals' argument.
So far these functions have only be used with fixture system start /slot lengths, but we can also use them in the withOfflineChain as it is currently using a consensus interpreter with a never forking summary.
As we have been using a neverForksSummary, the interpreter would never fail and the conversion is effectively the same as doing the math directly. Hence, the newly factored out functions slotNoFromUTCTime and slotNoToUTCTime should provide the same semantics and much more readable code.
2ecdd29
to
5043f08
Compare
5043f08
to
fdec8a4
Compare
a988986
to
00427a5
Compare
NOTE: The diff of this is big because it undoes a few unnecessary changes from #1118. See this diff for the actual changes of introducing offline-mode minimized by the refactoring of this branch.
🎅 Adds command line option tests to retain interface.
🎅 Defines a default for
--node-id
which makes the command line parser re-usable for offline-mode. There are other ways to do this, but all other options had defaults already.🎅 Avoids code duplication by modeling
data ChainConfig = Offline .. | Direct ..
🎅 Moves
--hydra-scripts-tx-id
intoDirectChainConfig
to simplify offline mode parameterization🎅 Simplifies conversion of
ShelleyGenesis
intoGlobals
🎅 Extended tests about restarting offline mode nodes and simplify simulated head opening of offline mode.
🎅 Simplify ticking logic and passed arguments to
withOfflineChain
by using projecting slot/time functions (factored from already existing code).cardano-api:internal
functions. Should be resolved in a different PR / upstream.