-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(protocol): add preconf slasher #19330
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
base: shasta_upgrade2
Are you sure you want to change the base?
Conversation
packages/protocol/contracts/layer1/preconf/impl/PreconfSlasher.sol
Outdated
Show resolved
Hide resolved
I feel I will also be including Preconf EOP violations in this same contract. I was inclining toward having a separate contract for EOPs, but now that I think about it, the preconfer would then need to opt into another contract just for EOP. This is inconvenient. |
packages/protocol/contracts/layer1/preconf/impl/PreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/impl/PreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/libs/LibPreconfConstants.sol
Outdated
Show resolved
Hide resolved
bytes32 blockHash; | ||
// `true` if this preconfer is not going to deliver anymore | ||
// preconfirmations after this block | ||
bool eop; // End-Of-Preconf flag |
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 know we briefly discussed this in our call, but I still prefer using an enum here. It allows us to explicitly mark a block as one of the following:
- 1: Start of a batch
- 2: End of a batch, but not end of preconfirmation
- 3: End of a batch and end of preconfirmation
- 0: All other blocks
By enforcing the correct marker for each block, it becomes easier to identify mistakes. Also, if we decide not to use markers 1 and 2, we don’t need to change the protocol at all. Using an enum gives us this flexibility.
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
packages/protocol/contracts/layer1/preconf/iface/IPreconfSlasher.sol
Outdated
Show resolved
Hide resolved
ITaikoInbox.BatchInfo batchInfo; | ||
// This is the BatchMetadata of the batch that contains the block at height X | ||
ITaikoInbox.BatchMetadata batchMetadata; | ||
// This is the BatchMetadata of the next batch that contains the block at height X + 1 |
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.
what if block X and block X+1 are in the same batch?
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.
nextBatchMetadata
stays empty in that case. I think the suggestion on keeping evidences separate will make this clearer.
This PR covers
Preconfirmation Structure
,Preconfirmation Violations
,Missing EOP
andInvalid EOP
sections of our LLD doc