You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.
The name implies that it's not just an event. It's actually an event depending on the state. Is that an indication that it shouldn't be an event? I tend to think so. This might apply to other events in this enum i.e. the ones with Prior or Post in their name.
There is only one place where we handle this event differently than the sibling LockConfirmed. It's in position_metrics::Cfd::apply, and the same could be achieved by inspecting the current state.
Current version:
match event.event{// ...LockConfirmed => Self{state:AggregatedState::Open,
..self},// ...LockConfirmedAfterFinality => Self{// This event is only appended if lock confirmation happens after we spent from lock// on chain. This is for special case where collaborative settlement is triggered// before lock is confirmed. In such a case the CFD is closedstate:AggregatedState::Closed,
..self},// ...}
This popped into my head whilst thinking about #2465. I imagine that a good solution to that problem will depend on changing our events (or at least using them differently), so it might be a good idea to rethink some of the states before we add new ones.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
This is the event:
https://github.com/itchysats/itchysats/blob/fcb95fbcd9c5d4e8b26e64073b765f24a804ca1a/model/src/cfd.rs#L360-L366
My arguments would be:
Prior
orPost
in their name.LockConfirmed
. It's inposition_metrics::Cfd::apply
, and the same could be achieved by inspecting the current state.Current version:
Example proposal:
This popped into my head whilst thinking about #2465. I imagine that a good solution to that problem will depend on changing our events (or at least using them differently), so it might be a good idea to rethink some of the states before we add new ones.
Beta Was this translation helpful? Give feedback.
All reactions