-
Notifications
You must be signed in to change notification settings - Fork 165
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
Conversation
There was a problem hiding this 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?
Co-authored-by: Alex <445306+anorth@users.noreply.github.com>
@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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type: Technical | |
type: FRC |
see reasoning here feedback welcome
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- 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.
- 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.
Co-authored-by: Jiaying Wang <42981373+jennijuju@users.noreply.github.com>
Co-authored-by: Jorge Soares <547492+jsoares@users.noreply.github.com>
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 💙 |
Related discussions at #501