Add PartialEq to errors compared in tests

[Hardeezah]

Summary

#645 asked for PartialEq on the error types so tests could compare errors directly. It was closed by #768, which added PartialEq only where every inner error type also implements it, and removed manual implementations that could never be fully correct. This PR completes that coverage and replaces the remaining .to_string() error comparisons in tests with real structural checks (the TODOs still referencing #645).

Changes

• payjoin/src/core/persist.rs
  • Derive PartialEq on PersistedError, ApiError, and InternalPersistedError.
  • Add an ErrorState: PartialEq bound to verify_sync/verify_async and assert_eq! the errors instead of comparing .to_string().
  • Fix a latent test bug the string comparison had masked: fatal_advance expected ApiError::Fatal, but the transition impl produces ApiError::FatalWithState`. The expectation now matches actual behavior.
• payjoin/src/core/receive/v1/error.rs
  • Derive PartialEq on RequestError and InternalRequestError. Every inner error type implements PartialEq, so this follows the rule established in #768.
• payjoin/tests/integration.rs
  • Replace the string comparison with a structural match on the Error::Protocol(ProtocolError::OriginalPayload(_)) variant, per the maintainer guidance in #768. The top-level receive Error hierarchy cannot implement PartialEq because inner types such as bitcoin::psbt::PsbtParseError and http::Error do not.

Checklist

• 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. The branch contains one commit that passes cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, RUSTDOCFLAGS="-D warnings" cargo doc, and the local
  test suites.

Disclosure: co-authored by opencode

[caarloshenriq]

Thanks for the contribution

Concept ACK c9630d1

It is worth confirming one point before the final ACK: the fatal_advance fix changes the expected result from ApiError::Fatal to ApiError::FatalWithState. This is explained as a latent bug that was masked by the comparison using .to_string() (since both variants likely display the same way, hiding the discrepancy). That reasoning makes sense, but the PR merely adjusts the test to match what the transition implementation currently produces; it does not verify whether FatalWithState is actually the intended behavior for this transition. Is it correct to propagate the ErrorState in this case, or should this scenario result in a simple Fatal error without state? If this is confirmed to be correct, I can proceed with the full ACK.

[Hardeezah]

@caarloshenriq Good question, and yes, carrying ErrorState is correct here. fatal_advance is a fatal error that still advances to a next state, so the state is part of the design. Commit 461b714 changed this on purpose and also updated the callers to consume the state. The old test could not tell Fatal and FatalWithState apart because both print the same text, so the assertion was stale, not the code.

[caarloshenriq]

ACK c9630d1

[DanGould]

This seems well considered to me and in line with C-COMMON-TRAITS and what rust-bitcoin is doing. Any reason not to do Eq derives as well?

Thank you.

ACK c9630d1