Dead code warning for 'SHARED_CONTEXT_TWO' in payjoin crate

[Cyber-Lord]

Is there an existing issue for this?

• I have searched the existing issues

Are you using payjoin-ffi?

• Yes I am

If so, which host language?

None

Did you enable any features on the crate?

I have clone rust-payjoin and after running "cargo test" on the latest "master" branch, the compiler emits a "dead_code" warning.

What version of the payjoin crate are you using?

payjoin v0.24.0

Current behavior

There's a warning when "cargo test" command is run. It indicates that there's a code that has not been used. I have tried cleaning it and run the test again and it passed. If no plan for it, I'll comment it out and open a PR

Expected behavior

I expect the test to pass and dead codes handled

Steps to reproduce

You can reproduce by cloning the repo and running "cargo build" and "cargo test"

Relevant log output

warning: static SHARED_CONTEXT_TWO is never used
--> payjoin/src/core/receive/v2/mod.rs:1036:23
|
1036 | pub(crate) static SHARED_CONTEXT_TWO: Lazy = Lazy::new(|| SessionContext {
| ^^^^^^^^^^^^^^^^^^
|
= note: #[warn(dead_code)] on by default

[nothingmuch]

I have tried cleaning it and run the test again and it passed. If no plan for it, I'll comment it out and open a PR

I've been meaning to look into this but haven't yet, i don't know what PR introduced it yet (should be easy to find with RUSTFLAGS="-D warnings" git bisect run cargo test, but make sure to try with both --all-features and --features v2 in addition to the default features)

It would be nice to know how this happened, i suspect either a test got refactored and this is no longer in use and therefore should be deleted, or (this appears to be the case) it needs to be feature gated according to the feature gated code that uses it (_multiparty feature)

[nothingmuch]

Also note that the reason this was not caught by CI is that payjoin/contrib/test.sh uses --all-features, so i'm pretty sure this just needs to be feature gated by _multiparty (probably by moving to the multiparty tests)

[nothingmuch]

587af2d is the commit that introduced both this lazy static and the warning, @benalleng does it make sense to just move this to the multiparty tests or do you think it's better to feature gate the declaration with _multiparty?

[benalleng]

Ahh yes, SHARED_CONTEXT_TWO was initially placed in /v2/mod.rs since SessionContext's fields are all private but I think it probably makes more sense to pub(crate) the fields and move SHARED_CONTEXT_TWO into multiparty as we never really would expect to use it outside of a multiparty session.

Gating it would be fine but it perhaps wouldn't make much sense for external devs looking for some clarification through tests.

[Cyber-Lord]

Thank you for clarifying the original intent behind placing SHARED_CONTEXT_TWO in /v2/mod.rs. I think moving the code to multiparty and making the SessionContext fields pub(crate) will work. Should I give it a try and revert back?

[arminsabouri]

This const was removed in #923