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

feat: ZKChain: Consensus Decentralization merged fixes #1054

Open
wants to merge 9 commits into
base: consensus-fix-review
Choose a base branch
from
20 changes: 9 additions & 11 deletions l2-contracts/contracts/ConsensusRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity 0.8.24;
import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable-v4/proxy/utils/Initializable.sol";
import {IConsensusRegistry} from "./interfaces/IConsensusRegistry.sol";
import {ZeroAddress} from "./errors/L2ContractErrors.sol";

/// @author Matter Labs
/// @custom:security-contact [email protected]
Expand Down Expand Up @@ -49,7 +50,7 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg

function initialize(address _initialOwner) external initializer {
if (_initialOwner == address(0)) {
revert InvalidInputNodeOwnerAddress();
revert ZeroAddress();
}
_transferOwnership(_initialOwner);
}
Expand Down Expand Up @@ -80,7 +81,7 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
_verifyInputBLS12_381PublicKey(_validatorPubKey);
_verifyInputBLS12_381Signature(_validatorPoP);
_verifyInputSecp256k1PublicKey(_attesterPubKey);
if ( _attesterWeight == 0) {
if (_attesterWeight == 0) {
revert ZeroAttesterWeight();
}
if (_validatorWeight == 0) {
Expand Down Expand Up @@ -169,10 +170,10 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
if (deleted) {
return;
}
_snapshotValidatorIfOutdated(_node);

_snapshotValidatorIfOutdated(node);
node.validatorLatest.active = false;

emit ValidatorDeactivated(_nodeOwner);
}

Expand Down Expand Up @@ -204,7 +205,7 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
return;
}

_snapshotValidatorIfOutdated(_node);
_snapshotValidatorIfOutdated(node);
node.validatorLatest.active = true;

emit ValidatorActivated(_nodeOwner);
Expand Down Expand Up @@ -307,10 +308,7 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
/// @dev Verifies that the node owner exists in the registry.
/// @param _nodeOwner The address of the node's owner whose attester public key will be changed.
/// @param _pubKey The new ECDSA public key to assign to the node's attester.
function changeAttesterKey(
address _nodeOwner,
Secp256k1PublicKey calldata _pubKey
) external onlyOwner {
function changeAttesterKey(address _nodeOwner, Secp256k1PublicKey calldata _pubKey) external onlyOwner {
_verifyInputSecp256k1PublicKey(_pubKey);
_verifyNodeOwnerExists(_nodeOwner);
(Node storage node, bool deleted) = _getNodeAndDeleteIfRequired(_nodeOwner);
Expand Down Expand Up @@ -497,7 +495,7 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg

function _verifyInputAddress(address _nodeOwner) private pure {
if (_nodeOwner == address(0)) {
revert InvalidInputNodeOwnerAddress();
revert ZeroAddress();
}
}

Expand Down
2 changes: 0 additions & 2 deletions l2-contracts/contracts/interfaces/IConsensusRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,8 @@ interface IConsensusRegistry {
error UnauthorizedOnlyOwnerOrNodeOwner();
error NodeOwnerExists();
error NodeOwnerDoesNotExist();
error NodeOwnerNotFound();
error ValidatorPubKeyExists();
error AttesterPubKeyExists();
error InvalidInputNodeOwnerAddress();
error InvalidInputBLS12_381PublicKey();
error InvalidInputBLS12_381Signature();
error InvalidInputSecp256k1PublicKey();
Expand Down
1 change: 1 addition & 0 deletions l2-contracts/hardhat.config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import "@matterlabs/hardhat-zksync-solc";
import "@matterlabs/hardhat-zksync-verify";
import "@nomicfoundation/hardhat-chai-matchers";
import "@matterlabs/hardhat-zksync-node";
import "@nomiclabs/hardhat-ethers";
import "hardhat-typechain";

Expand Down
2 changes: 2 additions & 0 deletions l2-contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"@matterlabs/hardhat-zksync-deploy": "^0.7.0",
"@matterlabs/hardhat-zksync-solc": "^0.3.15",
"@matterlabs/hardhat-zksync-verify": "^0.4.0",
"@matterlabs/hardhat-zksync-node": "^1.2.0",
"@matterlabs/zksync-contracts": "^0.6.1",
"@nomicfoundation/hardhat-chai-matchers": "^1.0.6",
"@nomicfoundation/hardhat-ethers": "^3.0.4",
Expand Down Expand Up @@ -36,6 +37,7 @@
"test:foundry": "forge test --zksync --gas-limit 2000000000",
"clean": "hardhat clean",
"test": "hardhat test",
"test-node": "hardhat node-zksync --tag 0.1.0-alpha.29",
"verify": "hardhat run src/verify.ts",
"deploy-testnet-paymaster-through-l1": "ts-node src/deploy-testnet-paymaster-through-l1.ts",
"deploy-force-deploy-upgrader-through-l1": "ts-node src/deploy-force-deploy-upgrader-through-l1.ts",
Expand Down
55 changes: 43 additions & 12 deletions l2-contracts/test/consensusRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ describe("ConsensusRegistry", function () {
// Prepare the node list.
const numNodes = 10;
for (let i = 0; i < numNodes; i++) {
const node = makeRandomNode(provider);
const nodeEntry = makeRandomNodeEntry(node, i);
const node = makeRandomNode();
const nodeEntry = makeRandomNodeEntry(node, i + 1);
nodes.push(node);
nodeEntries.push(nodeEntry);
}
Expand All @@ -73,11 +73,14 @@ describe("ConsensusRegistry", function () {
await (
await registry.add(
nodeEntries[i].ownerAddr,
true,
nodeEntries[i].validatorWeight,
nodeEntries[i].validatorPubKey,
nodeEntries[i].validatorPoP,
true,
nodeEntries[i].attesterWeight,
nodeEntries[i].attesterPubKey
nodeEntries[i].attesterPubKey,
{ gasLimit }
)
).wait();
}
Expand Down Expand Up @@ -130,30 +133,48 @@ describe("ConsensusRegistry", function () {
.connect(nonOwner)
.add(
ethers.Wallet.createRandom().address,
true,
0,
{ a: new Uint8Array(32), b: new Uint8Array(32), c: new Uint8Array(32) },
{ a: new Uint8Array(32), b: new Uint8Array(16) },
true,
0,
{ tag: new Uint8Array(1), x: new Uint8Array(32) },
{ gasLimit }
)
).to.be.reverted;
});

it("Should allow owner to deactivate", async function () {
it("Should allow owner to deactivate attester", async function () {
const nodeOwner = nodeEntries[0].ownerAddr;
expect((await registry.nodes(nodeOwner)).attesterLatest.active).to.equal(true);

await (await registry.connect(owner).deactivateAttester(nodeOwner, { gasLimit })).wait();
expect((await registry.nodes(nodeOwner)).attesterLatest.active).to.equal(false);

// Restore state.
await (await registry.connect(owner).activateAttester(nodeOwner, { gasLimit })).wait();
});

it("Should allow owner to deactivate validator", async function () {
const nodeOwner = nodeEntries[0].ownerAddr;
expect((await registry.nodes(nodeOwner)).validatorLatest.active).to.equal(true);

await (await registry.connect(owner).deactivate(nodeOwner, { gasLimit })).wait();
await (await registry.connect(owner).deactivateValidator(nodeOwner, { gasLimit })).wait();
expect((await registry.nodes(nodeOwner)).validatorLatest.active).to.equal(false);

// Restore state.
await (await registry.connect(owner).activate(nodeOwner, { gasLimit })).wait();
await (await registry.connect(owner).activateValidator(nodeOwner, { gasLimit })).wait();
});

it("Should not allow nonOwner, nonNodeOwner to deactivate attester", async function () {
const nodeOwner = nodeEntries[0].ownerAddr;
await expect(registry.connect(nonOwner).deactivateAttester(nodeOwner, { gasLimit })).to.be.reverted;
});

it("Should not allow nonOwner, nonNodeOwner to deactivate", async function () {
it("Should not allow nonOwner, nonNodeOwner to deactivate validator", async function () {
const nodeOwner = nodeEntries[0].ownerAddr;
await expect(registry.connect(nonOwner).deactivate(nodeOwner, { gasLimit })).to.be.reverted;
await expect(registry.connect(nonOwner).deactivateValidator(nodeOwner, { gasLimit })).to.be.reverted;
});

it("Should change validator weight", async function () {
Expand Down Expand Up @@ -209,13 +230,15 @@ describe("ConsensusRegistry", function () {
});

it("Should not allow to add a node with a validator public key which already exist", async function () {
const newEntry = makeRandomNodeEntry(makeRandomNode(), 0);
const newEntry = makeRandomNodeEntry(makeRandomNode(), 1);
await expect(
registry.add(
newEntry.ownerAddr,
true,
newEntry.validatorWeight,
nodeEntries[0].validatorPubKey,
newEntry.validatorPoP,
true,
newEntry.attesterWeight,
newEntry.attesterPubKey,
{ gasLimit }
Expand All @@ -224,13 +247,15 @@ describe("ConsensusRegistry", function () {
});

it("Should not allow to add a node with an attester public key which already exist", async function () {
const newEntry = makeRandomNodeEntry(makeRandomNode(), 0);
const newEntry = makeRandomNodeEntry(makeRandomNode(), 1);
await expect(
registry.add(
newEntry.ownerAddr,
true,
newEntry.validatorWeight,
newEntry.validatorPubKey,
newEntry.validatorPoP,
true,
newEntry.attesterWeight,
nodeEntries[0].attesterPubKey,
{ gasLimit }
Expand Down Expand Up @@ -284,7 +309,8 @@ describe("ConsensusRegistry", function () {
const entry = nodeEntries[idx];

// Deactivate attribute.
await (await registry.deactivate(entry.ownerAddr, { gasLimit })).wait();
await (await registry.deactivateAttester(entry.ownerAddr, { gasLimit })).wait();
await (await registry.deactivateValidator(entry.ownerAddr, { gasLimit })).wait();

// Verify no change.
expect((await registry.getAttesterCommittee()).length).to.equal(nodes.length);
Expand All @@ -301,7 +327,8 @@ describe("ConsensusRegistry", function () {
expect((await registry.getValidatorCommittee()).length).to.equal(nodes.length - 1);

// Restore state.
await (await registry.activate(entry.ownerAddr, { gasLimit })).wait();
await (await registry.activateAttester(entry.ownerAddr, { gasLimit })).wait();
await (await registry.activateValidator(entry.ownerAddr, { gasLimit })).wait();
await (await registry.commitAttesterCommittee({ gasLimit })).wait();
await (await registry.commitValidatorCommittee({ gasLimit })).wait();
});
Expand Down Expand Up @@ -332,9 +359,11 @@ describe("ConsensusRegistry", function () {
await (
await registry.add(
entry.ownerAddr,
true,
entry.validatorWeight,
entry.validatorPubKey,
entry.validatorPoP,
true,
entry.attesterWeight,
entry.attesterPubKey
)
Expand Down Expand Up @@ -426,9 +455,11 @@ describe("ConsensusRegistry", function () {
await (
await registry.add(
entry.ownerAddr,
true,
entry.validatorWeight,
entry.validatorPubKey,
entry.validatorPoP,
true,
entry.attesterWeight,
entry.attesterPubKey
)
Expand Down
4 changes: 2 additions & 2 deletions system-contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
},
"devDependencies": {
"@matterlabs/hardhat-zksync-chai-matchers": "^0.2.0",
"@matterlabs/hardhat-zksync-node": "^0.0.1-beta.7",
"@matterlabs/hardhat-zksync-node": "^1.2.0",
"@nomicfoundation/hardhat-chai-matchers": "^1.0.3",
"@nomiclabs/hardhat-ethers": "^2.0.0",
"@typechain/ethers-v5": "^2.0.0",
Expand Down Expand Up @@ -68,7 +68,7 @@
"verify-on-explorer": "hardhat run scripts/verify-on-explorer.ts",
"test": "yarn build:test-system-contracts && hardhat test --network zkSyncTestNode",
"test-no-build": "hardhat test --network zkSyncTestNode",
"test-node": "hardhat node-zksync --tag v0.0.1-vm1.5.0",
"test-node": "hardhat node-zksync --tag 0.1.0-alpha.29",
"test:bootloader": "cd bootloader/test_infra && cargo run"
}
}
2 changes: 1 addition & 1 deletion system-contracts/test/L1Messenger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe("L1Messenger tests", () => {
l1Messenger
.connect(knownCodeStorageAccount)
.requestBytecodeL1Publication(ethers.utils.hexlify(utils.hashBytecode(bytecode)), {
gasLimit: 230000000,
gasLimit: 2300000000,
})
)
.to.emit(l1Messenger, "BytecodeL1PublicationRequested")
Expand Down
Loading
Loading