Conversation
Pull Request Test Coverage Report for Build 14446204931Details
💛 - Coveralls |
payjoin/src/receive/v2/persist.rs
Outdated
| impl Receiver { | ||
| /// Loads a [`Receiver`] from the provided persister using the storage token. | ||
| pub fn load<P: Persister<Receiver>>( | ||
| token: P::Token, | ||
| persister: &P, | ||
| ) -> Result<Self, ImplementationError> { | ||
| persister.load(token).map_err(ImplementationError::from) | ||
| } | ||
| } |
There was a problem hiding this comment.
I wonder if splitting the implementation of a type across multiple files for organization is worth the increased complexity for navigating the code. There is a clippy lint for it though it's allowed by default https://rust-lang.github.io/rust-clippy/master/#multiple_inherent_impl
@DanGould thoughts?
There was a problem hiding this comment.
I prefer to typestate transitions in the file that defines the type. If it were a utility maybe I could come around to splitting impl but in this circumstance I prefer to unite the typestate impls in the main mod file. From<ReceiverToken> and the ReceiverToken in persist submod makes sense to me.
There was a problem hiding this comment.
I still don't understand why multiparty can't reuse send/persist.rs? Implementations look pretty much the same afaict
There was a problem hiding this comment.
It's got a different token. You wouldn't want to load one from the other table and vice versa
arminsabouri
force-pushed
the
persistence-followups
branch
from
1d1629f to
936ee20
Compare
Move persistence-related code out of `v2::Sender` and `v2::Receiver` into dedicated `persist.rs` files. This refactor helps declutter the core API and separates unrelated concerns. This change addresses a follow-up item from the [PR that introduced persistence](payjoin#552 (comment)).
These were indentical storage tokens and can be reconciled. This was a follow up item from the [PR that introduce persistence](payjoin#552 (comment))
arminsabouri
force-pushed
the
persistence-followups
branch
from
936ee20 to
744c661
Compare
|
Wait, what happened here in 744c661? If the SenderTokens are not distinct types then the comment in #638 (comment) becomes a problem because you can use the same token to get a different type from your DB. Should the tokens not have some differentiator? |
One way to think about it is that all senders start as v2 sessions. If there is an optimistic merge oppurtunity then they will evolve in to NS1R senders. But in this framework they would share the same storage token. Under the session event log frame work they also start as v2 senders and by replaying the session events they will also transition to a multi party sender that will expect an additional round of comms. Either way I can't think of a concrete reason why they need to be stored seperately. |
After removing the as_ref method for the RecieverToken in payjoin#638 we no longer needed an assert that prevented a mutant from cropping up. This was missed prior to push and caused some failing lints.
After removing the as_ref method for the RecieverToken in payjoin#638 we needed to modify an assert that prevented a mutant from cropping up. This was missed prior to push and caused some failing lints.
After removing the as_ref method for the RecieverToken in payjoin#638 we needed to modify an assert that prevented a mutant from cropping up. This was missed prior to push and caused some failing lints.
After removing the as_ref method for the RecieverToken in payjoin#638 we needed to modify an assert that prevented a mutant from cropping up. This was missed prior to push and caused some failing lints.
4 participants
Follow up items from #552