Fix mrsv lint warnings

[benalleng]

Closes #1743

This is less impactful than I first thought in the issue since the SessionContext is a private field

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]

Coverage Report for CI Build 29586100405

Coverage remained the same at 86.187%

Details

• Coverage remained the same as the base build.
• Patch coverage: No coverable lines changed in this PR.
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	15963
Covered Lines:	13758
Line Coverage:	86.19%
Coverage Strength:	345.63 hits per line

---

💛 - Coveralls

[caarloshenriq]

ACK

[benalleng]

I could also go with #[allow(clippy::large_enum_variant)] instead here

[spacebear21]

My preference would depend on whether Serialize/Deserialize is affected by the Box<> change. If it changes the persistence format then this is breaking change, which I'm not sure is worth it for an MSRV lint. I'm also curious why this particular lint is msrv-only, did clippy change the way it determines what constitutes a large variant?

[benalleng]

Looks like this is from rust-lang/rust-clippy#14717 which followed up rust-lang/rust-clippy#13833 so I don't think this is really a variable behavior just something that was/is buggy?

As for serialization / deserialization I believe that Box<> is transparent to serde. testing that now.

[benalleng]

I tested to confirm and the json outputs are identical from before and after e1f956a. And here is the source that i believe proves the behavior for serialization. https://docs.rs/serde_core/1.0.228/src/serde_core/ser/impls.rs.html#460-490

[spacebear21]

Cool. I'm still leaning towards ignoring that lint if it's being raised due to a bug in 1.85 anyway?

[benalleng]

Fair enough, since it's non-breaking we can always add this down the line. I am going to try and get an answer on the expected behavior for this lint and confirm that it's not broken now but that can be a follow-up

[benalleng]

Hmm this is especialy odd considering the sender SessionEvent does wrap SessionContext in a Box<> as the lint fails there even on nightly

[benalleng]

Ok, so not a bug, just how the lint works. It compares against the second largest variant in nightly and looks for a difference > 200 bytes as an enum is limited by the largest variant so a large size discrepancy leaves room for optimization but if all, or many, of the variants are large it doesn't matter. Whereas in older versions it just looks for any variant > 200 bytes. This is why the sender needs the Box even on nightly as there are not other equally large variants there.

rust-payjoin/payjoin/src/core/send/v2/session.rs

Lines 159 to 168 in 218d8ae

	pub enum SessionEvent {
	/// Sender was created with session data
	Created(Box<SessionContext>),
	/// Sender POSTed the Original PSBT and is waiting to receive a Proposal PSBT
	PostedOriginalPsbt(),
	/// User initiated cancellation of the session
	Cancelled(),
	/// Closed successful or failed session
	Closed(SessionOutcome),
	}

While the receiver has variants that contain the entire PSBT etc

rust-payjoin/payjoin/src/core/receive/v2/session.rs

Lines 196 to 212 in 218d8ae

	pub enum SessionEvent {
	Created(SessionContext),
	RetrievedOriginalPayload { original: OriginalPayload, reply_key: Option<crate::HpkePublicKey> },
	CheckedBroadcastSuitability(),
	CheckedInputsNotOwned(),
	CheckedNoInputsSeenBefore(),
	IdentifiedReceiverOutputs(Vec<usize>),
	CommittedOutputs { outputs: Vec<bitcoin::TxOut>, change_vout: usize },
	CommittedInputs { receiver_inputs: Vec<InputPair>, payjoin_psbt: bitcoin::Psbt },
	AppliedFeeRange(PsbtContext),
	FinalizedProposal(bitcoin::Psbt),
	GotReplyableError(JsonReply),
	PostedPayjoinProposal(),
	Cancelled,
	ProtocolFailed,
	Closed(SessionOutcome),
	}

Edit: this is wrong

[benalleng]

It was my first guess afterall with rust-lang/rust-clippy#14717 where is_normalizable returns false for some variants in this case likely bitcoin::Psbt. Because the receiver SessionEvent contains several variants relating to Psbts there was always going to be a large discrepancy as they would be of size 0 in 1.85.0. I made the assumption above that the lint became a comparator since 1.85.0 to now but it has been a comparator since before 1.63.0. The removal of the is_normalizable and related logic in nightly now calculate variant sizes more accurately.

[benalleng]

rebased with a TODO explaining the discrepancy needs a closer look

[spacebear21]

ACK