Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Method to list incomplete checkpoints #207

Merged
merged 18 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
433 changes: 251 additions & 182 deletions .storage-layouts/GatewayActorModifiers.json

Large diffs are not rendered by default.

431 changes: 250 additions & 181 deletions .storage-layouts/GatewayDiamond.json

Large diffs are not rendered by default.

188 changes: 94 additions & 94 deletions .storage-layouts/SubnetActorDiamond.json

Large diffs are not rendered by default.

188 changes: 94 additions & 94 deletions .storage-layouts/SubnetActorModifiers.json

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ install-eth-abi:
curl -sSL https://bootstrap.pypa.io/get-pip.py -o get-pip.py && python3 get-pip.py && rm get-pip.py && python3 -m pip install eth_abi

storage:
rm -rf ./cache
rm -rf ./cache_hardhat
npx hardhat storage-layout --update

clean:
Expand Down
1 change: 1 addition & 0 deletions src/GatewayDiamond.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ contract GatewayDiamond {
s.topDownCheckPeriod = params.topDownCheckPeriod;
s.crossMsgFee = params.msgFee;
s.majorityPercentage = params.majorityPercentage;
s.bottomUpCheckpointRetentionHeight = 1;

// the root doesn't need to be explicitly initialized
if (s.networkName.isRoot()) {
Expand Down
6 changes: 6 additions & 0 deletions src/errors/IPCErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ error CallerHasNoStake();
error CannotReleaseZero();
error CannotSendCrossMsgToItself();
error CheckpointAlreadyExists();
error CheckpointAlreadyProcessed();
error CheckpointInfoAlreadyExists();
error IncompleteCheckpointExists();
error FailedAddSignatory();
error FailedAddIncompleteCheckpoint();
error FailedRemoveIncompleteCheckpoint();
error CheckpointNotChained();
error CollateralIsZero();
error CollateralStillLockedInSubnet();
Expand Down Expand Up @@ -49,6 +54,7 @@ error NotSystemActor();
error NotRegisteredSubnet();
error NotValidator();
error PostboxNotExist();
error InvalidRetentionHeight();
error SignatureReplay();
error SubnetAlreadyKilled();
error SubnetNotActive();
Expand Down
35 changes: 34 additions & 1 deletion src/gateway/GatewayGetterFacet.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity 0.8.19;

import {CrossMsg, BottomUpCheckpoint, StorableMsg, ParentFinality, CheckpointInfo} from "../structs/Checkpoint.sol";
import {CrossMsg, BottomUpCheckpoint, BottomUpCheckpointNew, StorableMsg, ParentFinality, CheckpointInfo} from "../structs/Checkpoint.sol";
import {EpochVoteTopDownSubmission} from "../structs/EpochVoteSubmission.sol";
import {SubnetID, Subnet} from "../structs/Subnet.sol";
import {Membership} from "../structs/Validator.sol";
Expand All @@ -10,13 +10,15 @@ import {LibGateway} from "../lib/LibGateway.sol";
import {GatewayActorStorage} from "../lib/LibGatewayActorStorage.sol";
import {LibVoting} from "../lib/LibVoting.sol";
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol";
import {EnumerableSet} from "openzeppelin-contracts/utils/structs/EnumerableSet.sol";

contract GatewayGetterFacet {
// slither-disable-next-line uninitialized-state
GatewayActorStorage internal s;

using SubnetIDHelper for SubnetID;
using CheckpointHelper for BottomUpCheckpoint;
using EnumerableSet for EnumerableSet.UintSet;

function crossMsgFee() external view returns (uint256) {
return s.crossMsgFee;
Expand Down Expand Up @@ -194,4 +196,35 @@ contract GatewayGetterFacet {
function getCheckpointCurrentWeight(uint64 h) public view returns (uint256) {
return s.bottomUpCheckpointInfo[h].currentWeight;
}

/// @notice get the incomplete checkpoint heights
function getIncompleteCheckpointHeights() public view returns (uint256[] memory) {
return s.incompleteCheckpoints.values();
}

/// @notice get the incomplete checkpoints
function getIncompleteCheckpoints() public view returns (BottomUpCheckpointNew[] memory) {
uint256[] memory heights = s.incompleteCheckpoints.values();
uint256 size = heights.length;

BottomUpCheckpointNew[] memory checkpoints = new BottomUpCheckpointNew[](size);
for (uint64 i = 0; i < size; ) {
checkpoints[i] = s.bottomUpCheckpoints[uint64(heights[i])];
unchecked {
++i;
}
}
return checkpoints;
}

/// @notice get the bottom-up checkpoint retention index
function getBottomUpRetentionHeight() public view returns (uint64) {
return s.bottomUpCheckpointRetentionHeight;
}

/// @notice Calculate the threshold required for quorum in this subnet
/// based on the configured majority percentage and the total weight of the validators.
function getQuorumThreshold(uint256 totalWeight) public view returns (uint256) {
return LibGateway.weightNeeded(totalWeight, s.majorityPercentage);
}
}
63 changes: 55 additions & 8 deletions src/gateway/GatewayRouterFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import {IPCMsgType} from "../enums/IPCMsgType.sol";
import {SubnetID, Subnet} from "../structs/Subnet.sol";
import {IPCMsgType} from "../enums/IPCMsgType.sol";
import {Membership} from "../structs/Validator.sol";
import {InconsistentPrevCheckpoint, NotEnoughSubnetCircSupply, InvalidCheckpointEpoch, InvalidSignature, NotAuthorized, SignatureReplay} from "../errors/IPCErrors.sol";
import {InvalidCheckpointSource, InvalidCrossMsgNonce, InvalidCrossMsgDstSubnet, CheckpointAlreadyExists, CheckpointInfoAlreadyExists} from "../errors/IPCErrors.sol";
import {InconsistentPrevCheckpoint, NotEnoughSubnetCircSupply, InvalidCheckpointEpoch, InvalidSignature, NotAuthorized, SignatureReplay, InvalidRetentionHeight, FailedRemoveIncompleteCheckpoint} from "../errors/IPCErrors.sol";
import {InvalidCheckpointSource, InvalidCrossMsgNonce, InvalidCrossMsgDstSubnet, CheckpointAlreadyExists, CheckpointInfoAlreadyExists, IncompleteCheckpointExists, CheckpointAlreadyProcessed, FailedAddIncompleteCheckpoint, FailedAddSignatory} from "../errors/IPCErrors.sol";
import {MessagesNotSorted, NotInitialized, NotEnoughBalance, NotRegisteredSubnet} from "../errors/IPCErrors.sol";
import {NotValidator, SubnetNotActive, CheckpointNotCreated, CheckpointMembershipNotCreated, ZeroMembershipWeight} from "../errors/IPCErrors.sol";
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol";
Expand All @@ -25,6 +25,7 @@ import {FvmAddressHelper} from "../lib/FvmAddressHelper.sol";
import {FilAddress} from "fevmate/utils/FilAddress.sol";
import {ECDSA} from "openzeppelin-contracts/utils/cryptography/ECDSA.sol";
import {MerkleProof} from "openzeppelin-contracts/utils/cryptography/MerkleProof.sol";
import {EnumerableSet} from "openzeppelin-contracts/utils/structs/EnumerableSet.sol";

contract GatewayRouterFacet is GatewayActorModifiers {
using FilAddress for address;
Expand All @@ -34,6 +35,8 @@ contract GatewayRouterFacet is GatewayActorModifiers {
using CrossMsgHelper for CrossMsg;
using FvmAddressHelper for FvmAddress;
using StorableMsgHelper for StorableMsg;
using EnumerableSet for EnumerableSet.UintSet;
using EnumerableSet for EnumerableSet.AddressSet;

event QuorumReached(uint64 height, bytes32 checkpoint, uint256 quorumWeight);
event QuorumWeightUpdated(uint64 height, bytes32 checkpoint, uint256 newWeight);
Expand Down Expand Up @@ -188,9 +191,10 @@ contract GatewayRouterFacet is GatewayActorModifiers {
}
}

/// @notice checks whether the provided checkpoint signature for a block at height `h ` is valid and accumulates that it.
/// @notice checks whether the provided checkpoint signature for the block at height `height` is valid and accumulates that it
/// @dev If adding the signature leads to reaching the threshold, then the checkpoint is removed from `incompleteCheckpoints`
/// @param height - the height of the block in the checkpoint
/// @param membershipProof - a Merkle proof that the validator was in the membership at height `h`
/// @param membershipProof - a Merkle proof that the validator was in the membership at height `height` with weight `weight`
/// @param weight - the weight of the validator
/// @param signature - the signature of the checkpoint
function addCheckpointSignature(
Expand All @@ -199,6 +203,9 @@ contract GatewayRouterFacet is GatewayActorModifiers {
uint256 weight,
bytes memory signature
) external {
if (height < s.bottomUpCheckpointRetentionHeight) {
revert CheckpointAlreadyProcessed();
}
BottomUpCheckpointNew memory checkpoint = s.bottomUpCheckpoints[height];
if (checkpoint.blockHeight == 0) {
revert CheckpointNotCreated();
Expand All @@ -218,7 +225,7 @@ contract GatewayRouterFacet is GatewayActorModifiers {
}

// Check whether the validator has already sent a valid signature
if (s.bottomUpCollectedSignatures[height][recoveredSignatory]) {
if (s.bottomUpCollectedSignatures[height].contains(recoveredSignatory)) {
revert SignatureReplay();
}

Expand All @@ -233,12 +240,20 @@ contract GatewayRouterFacet is GatewayActorModifiers {
// All checks passed.
// Adding signature and emitting events.

s.bottomUpCollectedSignatures[height][recoveredSignatory] = true;
bool ok = s.bottomUpCollectedSignatures[height].add(recoveredSignatory);
if (!ok) {
revert FailedAddSignatory();
}
checkpointInfo.currentWeight += weight;

if (checkpointInfo.currentWeight >= checkpointInfo.threshold) {
if (!checkpointInfo.reached) {
checkpointInfo.reached = true;
// checkpoint is completed since the threshold has been reached
ok = s.incompleteCheckpoints.remove(checkpoint.blockHeight);
if (!ok) {
revert FailedRemoveIncompleteCheckpoint();
}
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
emit QuorumReached({
height: height,
checkpoint: checkpointHash,
Expand All @@ -263,20 +278,28 @@ contract GatewayRouterFacet is GatewayActorModifiers {
bytes32 membershipRootHash,
uint256 membershipWeight
) external systemActorOnly {
if (checkpoint.blockHeight < s.bottomUpCheckpointRetentionHeight) {
revert CheckpointAlreadyProcessed();
}
if (s.bottomUpCheckpoints[checkpoint.blockHeight].blockHeight > 0) {
revert CheckpointAlreadyExists();
}
if (s.bottomUpCheckpointInfo[checkpoint.blockHeight].threshold > 0) {
revert CheckpointInfoAlreadyExists();
}

if (membershipWeight == 0) {
revert ZeroMembershipWeight();
}

uint256 threshold = LibGateway.getThreshold(membershipWeight);
uint256 threshold = LibGateway.weightNeeded(membershipWeight, s.majorityPercentage);

// process the checkpoint
s.bottomUpCheckpoints[checkpoint.blockHeight] = checkpoint;

bool ok = s.incompleteCheckpoints.add(checkpoint.blockHeight);
if (!ok) {
revert FailedAddIncompleteCheckpoint();
}
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
s.bottomUpCheckpointInfo[checkpoint.blockHeight] = CheckpointInfo({
hash: checkpoint.toHash(),
rootHash: membershipRootHash,
Expand All @@ -285,4 +308,28 @@ contract GatewayRouterFacet is GatewayActorModifiers {
reached: false
});
}

/// @notice Set a new checkpoint retention height and garbage collect all checkpoints in range [`retentionHeight`, `newRetentionHeight`)
/// @dev `retentionHeight` is the height of the first incomplete checkpointswe must keep to implement checkpointing.
/// All checkpoints with a height less than `retentionHeight` are removed from the history, assuming they are committed to the parent.
/// @param newRetentionHeight - the height of the oldest checkpoint to keep
function pruneBottomUpCheckpoints(uint64 newRetentionHeight) external systemActorOnly {
uint64 oldRetentionHeight = s.bottomUpCheckpointRetentionHeight;

if (newRetentionHeight <= oldRetentionHeight) {
revert InvalidRetentionHeight();
}

for (uint64 h = oldRetentionHeight; h < newRetentionHeight; ) {
delete s.bottomUpCheckpoints[h];
delete s.bottomUpCheckpointInfo[h];
delete s.bottomUpCollectedSignatures[h];
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also clear the bottomUpCheckpointsLegacy while it exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those two bottom-up checkpoint types are probably misleading. But bottomUpCheckpointsLegacy is not used in implementing the new bottom-up checkpoint mechanism. It is in the code just to build it without removing all libraries, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I understand we want to get rid of it, just wanted to highlight that since I'm relying on it in consensus-shipyard/fendermint#286 due to the lack of any alternatives, technically it will have data in it as long as it exists and someone sends a bottom-up message this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be fixed in #214


unchecked {
++h;
}
}

s.bottomUpCheckpointRetentionHeight = newRetentionHeight;
}
}
8 changes: 4 additions & 4 deletions src/lib/LibGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ library LibGateway {
found = !subnet.id.isEmpty();
}

/// @notice returns the threshold corresponding to the majority percentage
function getThreshold(uint256 weight) internal view returns (uint256) {
GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
return (weight * s.majorityPercentage) / 100;
/// @notice returns the needed weight value corresponding to the majority percentage
/// @dev `majorityPercentage` must be a valid number
function weightNeeded(uint256 weight, uint256 majorityPercentage) internal pure returns (uint256) {
return (weight * majorityPercentage) / 100;
}
}
10 changes: 9 additions & 1 deletion src/lib/LibGatewayActorStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {SubnetID, Subnet} from "../structs/Subnet.sol";
import {Membership} from "../structs/Validator.sol";
import {AccountHelper} from "../lib/AccountHelper.sol";
import {FilAddress} from "fevmate/utils/FilAddress.sol";
import {EnumerableSet} from "openzeppelin-contracts/utils/structs/EnumerableSet.sol";

struct GatewayActorStorage {
/// @notice List of subnets
Expand Down Expand Up @@ -38,8 +39,15 @@ struct GatewayActorStorage {
/// @notice A mapping of block numbers to checkpoint data
// slither-disable-next-line uninitialized-state
mapping(uint64 => CheckpointInfo) bottomUpCheckpointInfo;
/// @notice The height of the first bottom-up checkpoint that must be retained since they have not been processed in the parent.
/// All checkpoint with the height less than this number may be garbage collected in the child subnet.
/// @dev Initial retention index is 1.
uint64 bottomUpCheckpointRetentionHeight;
/// @notice A list of incomplete checkpoints.
// slither-disable-next-line uninitialized-state
EnumerableSet.UintSet incompleteCheckpoints;
/// @notice The validators have already sent signatures at height `h`
mapping(uint64 => mapping(address => bool)) bottomUpCollectedSignatures;
mapping(uint64 => EnumerableSet.AddressSet) bottomUpCollectedSignatures;
/// @notice epoch => SubnetID => [childIndex, exists(0 - no, 1 - yes)]
mapping(uint64 => mapping(bytes32 => uint256[2])) children;
/// @notice epoch => SubnetID => check => exists
Expand Down
Loading
Loading