Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate HTTP(S) Transactions #54

Merged
merged 7 commits into from
Dec 1, 2020
Merged

Conversation

j01tz
Copy link
Member

@j01tz j01tz commented Apr 29, 2020

Copy link
Contributor

@lehnberg lehnberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job turning this around so quickly, @Joltz.
Couple of comments regarding the text, see separate comments.

On a more meta-level, in Tuesday's meeting you wrote:

We could just say: "it is unacceptable for grin to have default transaction methods that are not private by default" or something.

Did you consider writing an RFC on that topic instead? I.e. rather than the specific case against http(s), instead making a much more general case arguing for a ban on any current and future transaction building methods that are not privacy-preserving by default (and offering some definitions for what should be the criteria for such evaluations).

Such an RFC would have a longer "shelf life", and would be applicable to future methods as well. And then whether we keep/reject http(s) becomes more of an "implementation detail" rather than something we need to RFC specifically.

Just a thought. I also see the appeal for a more to the point worded proposal about http(s), still, I'm wondering whether it would be even more powerful if we formalise a general view on methods we want Grin to support.

text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
@j01tz
Copy link
Member Author

j01tz commented May 2, 2020

Thanks for the detailed review @lehnberg!

Did you consider writing an RFC on that topic instead?

The reasoning for giving this its own RFC was that since deprecating HTTP(S) is such a major change, it might need its own slot to get enough attention and to avoid catching anyone by surprise.

One idea is that once deprecate-http-tx, armored-slates, compact-slates, slate-serialization, slatepack-metadata etc RFCs are done we could converge on a "Standard Transaction" RFC that would be the minimum viable transaction that any wallet or service should support.

Beyond a "standard transaction", other optional methods could be used as long as they inform users of the risks. So assuming slatepack/armored slates are adopted as the "standard transaction" they would be supported by all wallets and services, with Tor (or any other method adopted) offered as an optional convenience.

I'm wondering whether it would be even more powerful if we formalise a general view on methods we want Grin to support.

I think this would be positive in general. It would help us prioritize acceptable tradeoffs when looking at possible future transaction methods. This might be more nebulous to iron out in the detail needed for a RFC as opposed to adding, removing or converging on a specific transaction method though.

Overall I'm not sure what is best: trying to push toward a path with a "standard transaction" that is universally supported or trying to push toward a path that may not have a universally supported transaction but doesn't have any options that are not acceptable according to higher order values like "privacy by default". Those aren't mutually exclusive either and the latter path may ultimately help forge the "standard transaction" path.

@lehnberg lehnberg added the wallet dev Related to wallet dev team label May 4, 2020
@ramheat
Copy link

ramheat commented Jul 20, 2020

core is still undecided about https?

@lehnberg
Copy link
Contributor

Assigning @Paouky as shepherd

Copy link
Contributor

@Paouky Paouky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(DIdn't mean to request changes, but I can't change it now).

users and services will be encouraged to use more privacy-friendly transaction methods like Tor or directly exchanging armored slates.

For the average user this means that to send or receive Grin they can use the Tor method or file exchange (potentially to be replaced with armored slates). It is no longer possible to send and receive Grin via the HTTP(S) method without custom configuration.

asynchronous exchange of armored slate strings or files

Should we update this to be coherent with the Slatepack terminology, for the sake of clarity when it's referenced in the future?

@ramheat
Copy link

ramheat commented Oct 8, 2020

(DIdn't mean to request changes, but I can't change it now).

users and services will be encouraged to use more privacy-friendly transaction methods like Tor or directly exchanging armored slates.

For the average user this means that to send or receive Grin they can use the Tor method or file exchange (potentially to be replaced with armored slates). It is no longer possible to send and receive Grin via the HTTP(S) method without custom configuration.

asynchronous exchange of armored slate strings or files

Should we update this to be coherent with the Slatepack terminology, for the sake of clarity when it's referenced in the future?

Kill the https!

@j01tz
Copy link
Member Author

j01tz commented Oct 11, 2020

I updated the RFC to reflect the Slatepack changes. Please let me know if anything could be clarified to help make this transition as easy as possible for developers and services to follow.

@Paouky
Copy link
Contributor

Paouky commented Oct 14, 2020

This RFC enters its final comment period (FCP).

Please voice any possible concerns during this 2 week period.

@lehnberg lehnberg added the in FCP Currently in Final Comment Period label Oct 14, 2020
@lehnberg
Copy link
Contributor

This RFC is due to be merged by October 28 unless there are significant comments and objections raised.

Copy link
Contributor

@lehnberg lehnberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just reviewed @j01tz, please see my question regarding v5.0.0 wallets above, might be worth while discussing with the usual suspects.

text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
text/0000-deprecate-http-tx.md Outdated Show resolved Hide resolved
@j01tz j01tz requested a review from Paouky November 20, 2020 16:02
@davidtavarez
Copy link

👍🏽

@cekickafa
Copy link

👍

@lehnberg lehnberg merged commit 62ed348 into mimblewimble:master Dec 1, 2020
@lehnberg
Copy link
Contributor

lehnberg commented Dec 1, 2020

In line with our governance process, this RFC has now been merged 🚀

@jaspervdm
Copy link
Contributor

@lehnberg @j01tz this PR only added the link to the README, it didnt actually add the RFC text itself.

@lehnberg lehnberg mentioned this pull request Dec 8, 2020
@lehnberg
Copy link
Contributor

lehnberg commented Dec 8, 2020

@jaspervdm fixed: #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in FCP Currently in Final Comment Period wallet dev Related to wallet dev team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants