Conversation
6 tasks
v0d1ch
approved these changes
May 28, 2025
Member
Author
|
@v0d1ch what do you think about
|
Contributor
|
@ch1bo I think the deposit workflow is fine in the spec - no more hand-waving needed :) |
github-project-automation
bot
moved this from In review 👀
to Done ✔
in ☕ Hydra Team Work
ch1bo
added a commit
to cardano-scaling/hydra
that referenced
this pull request
Jun 4, 2025
This is will be the last PR related to deposits that also fixes the "deposit used too early" problem identified in course of #1951 and related discussions. With all the prior work already done in #1969, #1974 and the refactorings of #1977 this was fairly straight forward. :alarm_clock: Breaking change on the network as I needed to change the `ReqSn` message. :alarm_clock: Needed to substantially increase the `defaultTTL` of re-enqueing inputs. This is needed because the `ReqSn` handling will wait for the deposit to become active (passes the `--deposit-period`) and needs to act on the `ReqSn` only then. This is problematic because the input queue is not (yet?) persisted and restarting the node will lose that state -> the deposit will become active, but an `AckSn` would not be sent. See #1999 for a follow-up to address this. :alarm_clock: The fact that we observe a `SlotNo` in `DepositObservation` requires a `TimeHandle` to convert it further to a `UTCTime` (in `convertObservation`). As the `hydra-chain-observer` does not have a `TimeHandle`, I decided to switch to using `HeadObservation` (the type from `hydra-tx`) instead of `OnChainTx` (the type from `hydra-node`) for the chain observer / `hydra-explorer` interface. This results in quite a lot of (backwards compatible) changes and this companion PR: cardano-scaling/hydra-explorer#47 TODO: - [x] spec changes cardano-scaling/hydra-formal-specification#18 - [x] make chain observers not break the explorer API --- * [x] CHANGELOG updated * [x] Documentation updated * [x] Haddocks updated or not needed * [ ] No new TODOs introduced or explained herafter - Two new TODOs in `Hydra.HeadLogic.onOpenNetworkReqSn` on things we're not sure that are needed or even missing from the implementation (not critical, but just inconsistent)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Changes to the off-chain protocol as motivated by cardano-scaling/hydra#1978