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.

Sign up for GitHub

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

Jump to bottom

Record messages for rebroadcast after signing #474

Closed
Stebalien opened this issue Jul 14, 2024 · 4 comments · Fixed by #691
Closed

Record messages for rebroadcast after signing #474

Stebalien opened this issue Jul 14, 2024 · 4 comments · Fixed by #691
Assignees
@masih
Milestone
Milestone 2: F3 H...

Comments

@Stebalien
Copy link
Member

We currently record them before signing. Unfortunately, this isn't safe it a participant changes from one client to the other. To be clear, changing from one client to another isn't safe in general but... this is extra unsafe.

This will also let us directly use the rebroadcast queue to fix #392.
@Stebalien
Copy link
Member Author

Doing this is going to be a bit tricky because gpbft never actually sees the signed messages. It probably needs a way to trigger rebroadcast by calling the host?

Honestly, I'm not a huge fan... Another solution is to have the host remember signed messages (e.g., with the WAL). Then, when GPBFT sends a message, the host would:

  1. Validate that the message is within the correct instance (doesn't go backwards).
  2. Check if it has signed the message before (in the WAL) and, if so, re-broadcast the already signed message.
  3. Finally, if it's new, sign, record, and send.

We might also be able to use strict message ordering to make this simpler. I.e., record the instance/round/phase of the last signed message and only sign strictly "newer" messages.

If we had the keys on the participants, this would be simpler: just have participants remember their last instance/round/phase and report them whenever selecting a client.

@masih
Copy link
Member

masih commented Jul 15, 2024

I am leaning towards a solution where singing is segregated from rebroadcast: gpbft asks Host to sign, gpbft asks host to broadcast signed messages. No templates. That would invert control over "what to broadcast" back to gpbft.

However, I am conscious this approach would mean a larger refactor.

@rjan90
Copy link
Contributor

rjan90 commented Sep 20, 2024

Once the integration of WAL is completed, this will be solved: #392

@masih masih assigned masih and unassigned Kubuxu Oct 1, 2024
@masih
Copy link
Member

masih commented Oct 1, 2024

Separated rebroadcast via WAL into #679. My aim is to get this done as part of the same issue.

masih added a commit that referenced this issue Oct 3, 2024
@masih
Rebroadcast already signed messages from WAL
1f9b080 
Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474
@masih masih mentioned this issue Oct 3, 2024
Merged
masih added a commit that referenced this issue Oct 3, 2024
@masih
Rebroadcast already signed messages from WAL
cfa2e0d 
Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474
masih added a commit that referenced this issue Oct 3, 2024
@masih
Rebroadcast already signed messages from WAL
a886904 
Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474
masih added a commit that referenced this issue Oct 3, 2024
@masih
Rebroadcast already signed messages from WAL
94fffbb 
Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474
masih added a commit that referenced this issue Oct 3, 2024
@masih
Rebroadcast already signed messages from WAL
eb66dd6 
Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2024
@masih
Rebroadcast already signed messages from WAL (#691)
3b40024 
* Rebroadcast already signed messages from WAL

Previously, rebroadcast functioned by keeping track of message templates
and calling `RequestBroadcast` again to re-broadcast them. This approach
can cause erroneous behaviour when messages are re-signed in conjunction
with participant churn.

To avoid this category of errors treat broadcast final: Every
broadcasted message is recorded in WAL and once recorded the same
message is used for re-broadcast; no re-signing. A dedicated
re-broadcast API is introduced to take instance, round and phase and use
it to then look up broadcast messages.

Note that as a result of the changes above, failures are also sicky. In
that, if signing of a message fails at the time of original broadcast,
future requests to rebroadcast it will have no effect.

The changes here also include a series of WAL functionality refactors to
make it easier to traverse write-ahead log entries.

Fixes #474

* Keep wal exactly as dumb as it is and index messages externally
@masih masih closed this as completed in #691 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@masih masih

Labels
None yet
Projects
F3
Status: Done
Milestone
Milestone 2: F3 Hardening and Mainnet...
Development

Successfully merging a pull request may close this issue.

Rebroadcast already signed messages from WAL filecoin-project/go-f3
4 participants
@masih @Stebalien @Kubuxu @rjan90