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

AppAck considered harmful in large busy groups #649

Closed
rohan-wire opened this issue May 5, 2022 · 8 comments
Closed

AppAck considered harmful in large busy groups #649

rohan-wire opened this issue May 5, 2022 · 8 comments

Comments

@rohan-wire
Copy link
Contributor

The way the spec is written now, clients need to implement all proposal types, and they must react to appropriately to all valid proposals, including AppAck. AppAck in a large, busy group could result in a situation where clients with longer latency connections to the DS (perhaps in a different country, using satellite, or using some wireless networks) might be effectively prevented from sending any messages, or worse could be prevented from sending AppAcks because their proposals are always a few hundred milliseconds late.

I think AppAcks still have a place is relatively small groups with low message volumes, but I have a few suggestions:

  1. Describe this issue either in the AppAck proposal section or in the security considerations section
  2. I think it is reasonable for a group to consider some proposal types invalid. If there is consensus with this position, I'd like to make this clear with an extra sentence or two. If that is not the consensus, then I will propose that only Add, Remove, and Update are assumed to be supported by each group, and other proposal types have to be explicitly added.
@tomleavy
Copy link
Contributor

tomleavy commented May 5, 2022

My team had similar comments. I agree that this shouldn't be a 100% necessary thing to support.

@bifurcation
Copy link
Collaborator

I would differentiate between "support" in the sense of "if you receive one, do something sensible with it" vs. "send this under some circumstances". My understanding was that the requirement here was the former, and even that the definition of "sensible" was up to the implementation. The thinking being that if an application wanted to get some anti-suppression properties, it could send and verify these on some schedule, but when exactly they would be sent would be up to the application. Does that alleviate some of the concern here? If so, I would totally be open to clarifying that.

To @rohan-wire's point about latency, it seems like we could add an epoch parameter to AppAck so that you could report on past epochs' messages in a future epoch.

On considering proposal types invalid -- I think there has always been a notion that applications might have policies on which proposals are acceptable. Not just at the proposal type level, but at even finer granularities. Think about things like groups where only certain members are allowed to add/remove participants. I would also be fine laying this out more explicitly, though maybe it would be a more natural fit in the architecture document.

@bifurcation
Copy link
Collaborator

Sorry, meant to delete a comment, closed the issue instead.

@Bren2010
Copy link
Collaborator

Bren2010 commented May 5, 2022

In the past I've expressed reservations about AppAck being included in the spec. I don't think it's sufficiently general (it only handles a one specific pattern of message loss and won't work for others, per @rohan-wire's comment), and I don't like that we don't specify how to handle dropped messages in any way. The AppAck functionality is likely to just be re-invented at the application-level because of this. For example,

  • AppAck can't be used to implement read receipts
  • It doesn't help detect the suppression of handshake messages
  • It doesn't handle cases where some messages must be delivered and others may be dropped

So instead I would propose removing AppAck from the spec.

@Bren2010
Copy link
Collaborator

Bren2010 commented May 5, 2022

I've opened a PR to that effect in #654.

@bifurcation
Copy link
Collaborator

Virtual interim 2022-05-19

  • Agreement that the bar for support is quite low
  • ... but you don't get a security property unless members actually do something more than the minimum
  • That sort of implies that you want the group to opt in to this mechanism
  • So we should have a new proposal type in a separate I-D

@bifurcation
Copy link
Collaborator

(To be clear, AppAck should move from the main spec here to its own I-D.)

@bifurcation
Copy link
Collaborator

Closed by #654

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants