[spacebear21]

Follow up to discussion in #1106 (comment). Note that I stuck with past tense over imperative mood because IMO that indicates more clearly that this event happen-ed (e.g. RetrievedOriginalPayload would be difficult to express in imperative mood), also we already use past tense for Created and Closed.

SessionEvent variants should describe "what event happened" instead of "what this event produces". This makes the inner values less ambiguous and better abides by the principle that events only capture primary information.

We already do this for the Created and Closed variants. SessionInvalid was left as-is because it will be replaced soon.

Pull Request Checklist

Please confirm the following before requesting review:

• I have disclosed my use of
  AI
  in the body of this PR.
• I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

[coveralls]

Pull Request Test Coverage Report for Build 18106191436

Details

• 92 of 92 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 84.579%

---

Totals
Change from base Build 18045273466:	0.01%
Covered Lines:	8589
Relevant Lines:	10155

---

💛 - Coveralls

[arminsabouri]

cACK af66471

[DanGould]

I'd really like to see verbs in Imperative mood here. An event is only past tense once it's actually been applied.

Scratch that. Past tense makes sense here because they're events that happened, not commands. Names make sense as-is.

[DanGould]

As discussed offline, fn apply_... names still need updates

For example apply_maybe_inputs_owned must become apply_checked_broadcastable (Or really apply_checked_broadcast_suitability if we're keeping congruence with the check method names)

[spacebear21]

Changes since last review:

• I changed the names of the variants CheckedBroadcastSuitability and CheckedNoInputsSeenBefore to match the transition method names.
• The second commit updates the apply_event transition methods.

Note that CommittedFees doesn't match the method name apply_fee_range. This is intentional because I believe we should separate fee application from fee commitment similar to how we do in WantsOutputs/WantsInputs as discussed in #1108. I changed this to AppliedFeeRange to be consistent, per the discussion on dev call we likely won't use a commit_fees method.

[DanGould]

ACK 7586c53 LGTM