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

IPC-304: add permissioned flag to disable validator changes #305

Merged
merged 7 commits into from
Dec 5, 2023
Merged

IPC-304: add permissioned flag to disable validator changes #305

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d9b41b9
IPC-304: add permissioned flag to disable validator changes
adlrocha Dec 4, 2023
67abb24
Apply Prettier formatting
adlrocha Dec 4, 2023
aba8632
IPC-304: allow leave when subnet not bootstrapped
adlrocha Dec 5, 2023
37c8fd7
Merge branch 'ipc-204-permissioning' of github.com:consensus-shipyard…
adlrocha Dec 5, 2023
724e396
commit rust binding
auto-commit Dec 5, 2023
9bea1ae
commit ABI artifacts
auto-commit Dec 5, 2023
a86b398
IPC-304: update subnet actor deployment
adlrocha Dec 5, 2023
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
2,116 changes: 1,058 additions & 1,058 deletions .storage-layouts/GatewayActorModifiers.json
View file
Open in desktop

Large diffs are not rendered by default.

2,116 changes: 1,058 additions & 1,058 deletions .storage-layouts/GatewayDiamond.json
View file
Open in desktop

Large diffs are not rendered by default.

1,584 changes: 796 additions & 788 deletions .storage-layouts/SubnetActorDiamond.json
View file
Open in desktop

Large diffs are not rendered by default.

1,584 changes: 796 additions & 788 deletions .storage-layouts/SubnetActorModifiers.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/SubnetActorDiamond.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ contract SubnetActorDiamond {
uint16 activeValidatorsLimit;
uint256 minCrossMsgFee;
int8 powerScale;
bool permissioned;
Copy link
Contributor

@dnkolegov dnkolegov Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Technically, it is not permissioned. Here we do not allow to join even if the validator is "permissioned". I do not know the correct term for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

static is probably the accurate term, but not too descriptive. staticMembership ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, static sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I went with permissioned is because this is a first stage towards this design for federated validation: https://www.notion.so/pl-strflt/Federated-validation-6290874b58904818ac4fdddb5128d01b

I have no strong opinions, I can name this static until we have the permissioned feature.

}

constructor(IDiamond.FacetCut[] memory _diamondCut, ConstructorParams memory params) {
Expand Down Expand Up @@ -70,6 +71,7 @@ contract SubnetActorDiamond {
s.powerScale = params.powerScale;
s.minCrossMsgFee = params.minCrossMsgFee;
s.currentSubnetHash = s.parentId.createSubnetId(address(this)).toHash();
s.permissioned = params.permissioned;

s.validatorSet.activeLimit = params.activeValidatorsLimit;
// Start the next configuration number from 1, 0 is reserved for no change and the genesis membership
Expand Down
1 change: 1 addition & 0 deletions src/errors/IPCErrors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,4 @@ error FacetCannotBeZero();
error WrongGateway();
error CannotFindSubnet();
error UnknownSubnet();
error MethodNotAllowed();
2 changes: 2 additions & 0 deletions src/lib/LibSubnetActorStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity 0.8.19;

Check warning on line 2 in src/lib/LibSubnetActorStorage.sol

View workflow job for this annotation

GitHub Actions / Solhint check 

Found more than One contract per file. 2 contracts found!

import {ConsensusType} from "../enums/ConsensusType.sol";
import {NotGateway, SubnetAlreadyKilled} from "../errors/IPCErrors.sol";
Expand Down Expand Up @@ -43,6 +43,8 @@
ConsensusType consensus;
/// @notice Determines if the subnet has been bootstrapped (i.e. it has been activated)
bool bootstrapped;
/// @notice Determines if the subnet is permissionless or not
bool permissioned;
/// @notice Determines if the subnet has been successfully killed
bool killed;
// =========== Staking ===========
Expand Down
28 changes: 26 additions & 2 deletions src/subnet/SubnetActorManagerFacet.sol
View file
Open in desktop
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 {SubnetAlreadyBootstrapped, NotEnoughFunds, CollateralIsZero, CannotReleaseZero, NotOwnerOfPublicKey, EmptyAddress, NotEnoughBalance, NotEnoughBalanceForRewards, NotEnoughCollateral, NotValidator, NotAllValidatorsHaveLeft, NotStakedBefore, InvalidSignatureErr, InvalidCheckpointEpoch, InvalidCheckpointMessagesHash, InvalidPublicKeyLength} from "../errors/IPCErrors.sol";
import {SubnetAlreadyBootstrapped, NotEnoughFunds, CollateralIsZero, CannotReleaseZero, NotOwnerOfPublicKey, EmptyAddress, NotEnoughBalance, NotEnoughBalanceForRewards, NotEnoughCollateral, NotValidator, NotAllValidatorsHaveLeft, NotStakedBefore, InvalidSignatureErr, InvalidCheckpointEpoch, InvalidCheckpointMessagesHash, InvalidPublicKeyLength, MethodNotAllowed} from "../errors/IPCErrors.sol";
import {IGateway} from "../interfaces/IGateway.sol";
import {ISubnetActor} from "../interfaces/ISubnetActor.sol";
import {BottomUpCheckpoint, CrossMsg} from "../structs/Checkpoint.sol";
Expand Down Expand Up @@ -136,6 +136,12 @@ contract SubnetActorManagerFacet is ISubnetActor, SubnetActorModifiers, Reentran
/// @notice method that allows a validator to join the subnet
/// @param publicKey The off-chain 65 byte public key that should be associated with the validator
function join(bytes calldata publicKey) external payable nonReentrant notKilled {
// adding this check to prevent new validators from joining
// after the subnet has been bootstrapped. We will increase the
// functionality in the future to support explicit permissioning.
if (s.bootstrapped && s.permissioned) {
revert MethodNotAllowed();
}
if (msg.value == 0) {
revert CollateralIsZero();
}
Expand Down Expand Up @@ -180,6 +186,11 @@ contract SubnetActorManagerFacet is ISubnetActor, SubnetActorModifiers, Reentran

/// @notice method that allows a validator to increase its stake
function stake() external payable notKilled {
// disbling validator changes for permissioned subnets (at least for now
// until a more complex mechanism is implemented).
if (s.permissioned) {
revert MethodNotAllowed();
}
if (msg.value == 0) {
revert CollateralIsZero();
}
Expand All @@ -199,6 +210,11 @@ contract SubnetActorManagerFacet is ISubnetActor, SubnetActorModifiers, Reentran
/// @notice method that allows a validator to unstake a part of its collateral from a subnet
/// @dev `leave` must be used to unstake the entire stake.
function unstake(uint256 amount) external notKilled {
// disbling validator changes for permissioned subnets (at least for now
// until a more complex mechanism is implemented).
if (s.permissioned) {
revert MethodNotAllowed();
}
if (amount == 0) {
revert CannotReleaseZero();
}
Expand All @@ -223,8 +239,16 @@ contract SubnetActorManagerFacet is ISubnetActor, SubnetActorModifiers, Reentran
/// @dev it also return the validators initial balance if the
/// subnet was not yet bootstrapped.
function leave() external notKilled nonReentrant {
// remove bootstrap nodes added by this validator
// disbling validator changes for permissioned subnets (at least for now
// until a more complex mechanism is implemented).
// This means that initial validators won't be able to recover
// their collateral ever (worth noting in the docs if this ends
// up sticking around for a while).
if (s.permissioned) {
Copy link

Choose a reason for hiding this comment

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

  1. Probably I missed the discussion but why are we blocking leave as well?
  2. If we decide to do that, shouldn't we enforce it only for the already boostrapped network to keep it symmetrical with join (maybe not important in practice but easier to reason about IMO)

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're limiting membership changes to reduce the changes of things going wrong, it makes sense to limit them both ways and just not have to worry about those paths. Don't see a lot of value in keeping leave as presumably you're not starting with a large validator set that you can afford to lose over time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably I missed the discussion but why are we blocking leave as well?

We don´t want any validator change for now, as this is what is breaking Mycelium :)

If we decide to do that, shouldn't we enforce it only for the already boostrapped network to keep it symmetrical with join (maybe not important in practice but easier to reason about IMO)

This is a good point, fixed it

revert MethodNotAllowed();
}

// remove bootstrap nodes added by this validator
uint256 amount = LibStaking.totalValidatorCollateral(msg.sender);
if (amount == 0) {
revert NotValidator(msg.sender);
Expand Down
3 changes: 2 additions & 1 deletion test/GatewayDiamond.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ contract GatewayActorDiamondTest is StdInvariant, Test {
majorityPercentage: DEFAULT_MAJORITY_PERCENTAGE,
activeValidatorsLimit: 100,
powerScale: 12,
minCrossMsgFee: CROSS_MSG_FEE
minCrossMsgFee: CROSS_MSG_FEE,
permissioned: false
});

saManager = new SubnetManagerTestUtil();
Expand Down
2 changes: 2 additions & 0 deletions test/SubnetActorDiamond.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ contract SubnetActorDiamondTest is Test {
majorityPercentage: DEFAULT_MAJORITY_PERCENTAGE,
activeValidatorsLimit: 100,
powerScale: 12,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
}),
address(saDupGetterFaucet),
Expand Down Expand Up @@ -1332,6 +1333,7 @@ contract SubnetActorDiamondTest is Test {
majorityPercentage: _majorityPercentage,
activeValidatorsLimit: 100,
powerScale: 12,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
})
);
Expand Down
4 changes: 4 additions & 0 deletions test/SubnetRegistry.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ contract SubnetRegistryTest is Test {
majorityPercentage: DEFAULT_MAJORITY_PERCENTAGE,
activeValidatorsLimit: 100,
powerScale: DEFAULT_POWER_SCALE,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
});
vm.expectRevert(WrongGateway.selector);
Expand All @@ -285,6 +286,7 @@ contract SubnetRegistryTest is Test {
majorityPercentage: DEFAULT_MAJORITY_PERCENTAGE,
activeValidatorsLimit: 100,
powerScale: DEFAULT_POWER_SCALE,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
});
registerSubnetFacet.newSubnetActor(params);
Expand All @@ -304,6 +306,7 @@ contract SubnetRegistryTest is Test {
majorityPercentage: DEFAULT_MAJORITY_PERCENTAGE,
activeValidatorsLimit: 100,
powerScale: DEFAULT_POWER_SCALE,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
});
registerSubnetFacet.newSubnetActor(params);
Expand Down Expand Up @@ -343,6 +346,7 @@ contract SubnetRegistryTest is Test {
majorityPercentage: _majorityPercentage,
activeValidatorsLimit: 100,
powerScale: _powerScale,
permissioned: false,
minCrossMsgFee: CROSS_MSG_FEE
});
registerSubnetFacet.newSubnetActor(params);
Expand Down
Loading