Closed vu3rdd closed 1 year ago
Patch coverage: 60.07
% and project coverage change: +1.00
:tada:
Comparison is base (
e6ddc75
) 44.21% compared to head (008db1f
) 45.21%.:exclamation: Current head 008db1f differs from pull request most recent head ff78cbb. Consider uploading reports for the commit ff78cbb to get more accurate results
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
These are some questions raised by @piegamesde which we talked about and the solutions we agreed on:
The core module knows as little as possible about Dilation, because Dilation generally is opt-in (only thing it needs to know is to tell the other side "no I don't Dilate" if necessary, but even that should be a no-op because of backwards compatibility …)
can_dilate
and dilation_abilities
.
dilate()
Dilation knows nothing about file transfer, because it may be used as the basis for any application protocol
FileTransferV2Mode
in dilation/mod.rs
(Defining modes of file transfer application), AppVersions
(in dilation/mod.rs
) references DilatedTransfer
(references FileTransferV2Mode
)
dilation
so that we only use the public API of dilation. This can be merged with the upcoming file transfer v2 module later.Applications who use file transfer do not have to interact with Dilation. They just say "I want v2" and stuff just happens for them
Dilation generally is opt-in (only thing it needs to know is to tell the other side "no I don't Dilate" if necessary
There's no reason to say "I don't do Dilation": if a peer doesn't indicate support for something, it doesn't support it.
Finding: Wormhole object has method dilate() Encapsulate within feature flag
Applications will still want to opt-in to Dilation at runtime.
That is, plenty of "wormhole-using" applications may have no use or need for Dilation (e.g. magic-folder invites never use a bulk-data connection) so it seems you'd still want both things to me (a feature-flag, but also a dilate()
or similar method for the application to signal "I actually want to Dilate this wormhole").
There's no reason to say "I don't do Dilation": if a peer doesn't indicate support for something, it doesn't support it.
You're totally right, that was just bad wording.
Applications will still want to opt-in to Dilation at runtime.
This is covered by enabling the dilation feature flag and then (not) calling the dilate()
method. Without the dilation feature, that method simply won't exist then.
The CI failure may be nightly, but since there has been a new stable release since this was committed it might fail there too.
A quick dig at dependencies' issues tells me that updating the dependencies / cargo update
should fix the CI failure.
Force pushed the compacted branch. The original branch is still available: https://github.com/LeastAuthority/magic-wormhole.rs/tree/dilation-1-original
These commits add basic types etc for the manager state machine, makes dilation a command line option, refactors some non-dilation code a bit. The client can exchange
please
message with the python dilation client and is able to receiveconnection-hints
message.The plan is to send PRs so that we minimize the size of the subsequent PRs and make it ready for review and merge them as the development progresses rather than wait till the end and send a huge PR that would be hard to review.
Keep in mind that we (@ewanas, @vu3rdd, @wuan) are rust newbies, so expect parts of the code to be non-idiomatic.