onflow/flow-core-contracts

[DKG whiteboard] Restriction on broadcast

tarakby opened this issue · 4 comments

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.

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)

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.

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

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