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

FRC-0051: Create FIP: Adding (Synchronous) Consistent Broadcast to EC #527

Merged
merged 13 commits into from
Dec 28, 2022
Merged

Conversation

guy-goren
Copy link
Contributor

Related discussions at #501

Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

Regarding the title, I think we should generally avoid value judgements in the proposal title and aim to be strictly declarative, so "Improving" is a problem here. Could we call it something like "Synchronous Consistent Block Broadcast" or similar?

FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
@guy-goren guy-goren changed the title Create FIP: Improving EC security with Consistent Broadcast Create FIP: Adding (Synchronous) Consistent Broadcast to EC Nov 21, 2022
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
@jennijuju
Copy link
Member

jennijuju commented Nov 22, 2022

@filecoin-project/fips-editors qq: the proposed improvement is not consensus critical nor requires a network upgrade, it's mostly an implementation change that we'd like SPs to adopt. Does that make this an FRC more than an FIP?

FIPS/fip-nnn.md Outdated Show resolved Hide resolved
FIPS/fip-nnn.md Outdated
author: Guy Goren <guy.goren@protocol.ai>, Alfonso de la Rocha <alfonso@protocol.ai>
discussions-to: https://github.com/filecoin-project/FIPs/discussions/501
status: Draft
type: Technical
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type: Technical
type: FRC

see reasoning here feedback welcome

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, one reason for choosing the synchronous CBcast option was the fact that it is not "consensus breaking" (does not cause a fork even if not all miners adopt it -- in classic blockchain terminology). Hence, it can also fit as an FRC.
What is important to notice is that the security of the entire network improves when more SPs adopt it. Therefore, I suggest that even if it goes to the FRC track, it would be implemented in the next network upgrade in order to boost adoption.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have strong feelings but would like to clarify this in principle. Beyond the inconsistency addressed in #552, there's the fact that FRC, by analogy to Ethereum, has an application-layer connotation (which is also true of the existing FRCs).

We all agree this isn't consensus breaking. The questions are:

  1. It's clear that any consensus-breaking change requires a non-informational FIP. I don't read the language in FIP-0001 as requiring that all FIPs be consensus-breaking.
  2. An informational/FRC FIP, again per FIP-0001, "does not propose a new feature" and "do[es] not necessarily represent ... a recommendation". I would claim neither is true here.

Provided that we're consistent about this, I don't see an issue with it being an FRC. But, as Guy said, the security of the network is improved by a large majority of nodes implementing CB, and so it would be preferable to attain community consensus on the direction even if it's fully interoperable.

@jennijuju jennijuju changed the title Create FIP: Adding (Synchronous) Consistent Broadcast to EC FXX-0051: Create FIP: Adding (Synchronous) Consistent Broadcast to EC Nov 22, 2022
guy-goren and others added 2 commits November 25, 2022 10:14
Co-authored-by: Jiaying Wang <42981373+jennijuju@users.noreply.github.com>
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
Co-authored-by: Jorge Soares <547492+jsoares@users.noreply.github.com>
FIPS/fip-nnn.md Outdated Show resolved Hide resolved
@jennijuju jennijuju changed the title FXX-0051: Create FIP: Adding (Synchronous) Consistent Broadcast to EC FRC-0051: Create FIP: Adding (Synchronous) Consistent Broadcast to EC Dec 28, 2022
@jennijuju
Copy link
Member

I am merging this draft as it has been ready for a long time. I still think it is an FRC atm.

@kaitlin-beegle we shall update FIP-0001 soon with better definiation bettwen frc and fips. If needed, we can update the type of this one later.

@ adlrocha Thank you for the lotus impl 💙
@guy-goren @jsoaresAFAIK, the team is still doing more analysis with more aggressive parameters than 6 sec - once the team confirms the value, please update the FRC, and/or let the FIP editors know whenever its ready for last call!

@jennijuju jennijuju merged commit 92c721e into filecoin-project:master Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants