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

[DKG whiteboard] Restriction on broadcast #283

Open
tarakby opened this issue Mar 24, 2022 · 4 comments
Open

[DKG whiteboard] Restriction on broadcast #283

tarakby opened this issue Mar 24, 2022 · 4 comments
Assignees
Labels
Needs Estimation SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board

Comments

@tarakby
Copy link
Contributor

tarakby commented Mar 24, 2022

Context

The contract allows participating nodes to broadcast as many and as long data as they want. This issue is about adding restrictions to the messages sent on the whiteboard to avoid malicious behaviour:

  • restrict the size of messages sent to the whiteboard.
  • restrict the number of submissions per node if possible (to discuss with @jordanschalm).

Definition of Done:

  • @tarakby

    • Once the format of broadcast messages is updated, compute the possible sizes of broadcast messages (verification vector/complaint/complaint answer)
    • Compute the number of broadcasts possible per node.
  • @joshuahannan

    • rename the contract file to FlowDKGWhiteboard to avoid the confusion of thinking we're implementing DKG logic on chain. Renaming is not possible, a clarifying comment will be added instead.
    • update the contract to only accept certain sizes of broadcasts to the whiteboard (this is not about the size of final submissions which seems to be checked already)
    • [if confirmed] update the contract to accept a maximum number of broadcast per node per DGK instance.
@joshuahannan
Copy link
Member

rename the contract file to FlowDKGWhiteboard

AFAIK, we won't be able to change the name of the contract because that is an incompatible upgrade

update the contract to only accept certain sizes of broadcasts to the whiteboard (this is not about the size of final submissions which seems to be checked already)

This should be fine to upgrade

update the contract to accept a maximum number of broadcast per node per DGK instance.

This'll be difficult because it would require adding a new field to the contract, which is an incompatible upgrade. There is a workaround for this, but it is pretty awkward. What is limit were you thinking about setting? And is there another way to enforce it? (Transaction fees would prevent a node sending too many, but I don't know what your limit would be)

@tarakby
Copy link
Contributor Author

tarakby commented Mar 25, 2022

AFAIK, we won't be able to change the name of the contract because that is an incompatible upgrade

I only meant the name of the cdc file in the repo.

This'll be difficult because it would require adding a new field to the contract, which is an incompatible upgrade.

It's not sure we would like to implement this, as our code may send some transactions twice to the whiteboard and we don't want to censor the last messages. Let's wait for Jordan and I to confirm the need for this first before thinking further.
The limit would be something like 270 in mature Flow.

@joshuahannan
Copy link
Member

I only meant the name of the cdc file in the repo.

I don't like having contract file names that are different than the contract names themselves, so I would prefer to avoid this. I don't think it'll make much of a difference

@tarakby
Copy link
Contributor Author

tarakby commented Mar 25, 2022

Fair enough. I'll make a PR to add clarifying comments about the contract is used for in DKG. Issue description updated.

@joshuahannan joshuahannan added the SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Estimation SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board
Projects
Status: 🧊 Backlog
Development

No branches or pull requests

4 participants