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: adding a restriction to proposers and challenger 🤌 #550

Merged
merged 14 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
24 changes: 17 additions & 7 deletions config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ module.exports = {
// this is the etherum private key for accounts[0]
privateKey: '0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69e',
gas: 10000000,
gasCosts: 9000000000000000,
gasCosts: 80000000000000000,
fee: 1,
BLOCK_STAKE: 1, // 1 wei
bond: 10, // 10 wei
Expand All @@ -215,11 +215,21 @@ module.exports = {
erc20default: 2000,
},
},
RESTRICTIONS: [
{
name: 'MockERC20',
address: '0xB5Acbe9a0F1F8B98F3fC04471F7fE5d2c222cB44',
amount: 200,
RESTRICTIONS: {
signingKeys: {
bootProposerKey: '0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69d',
bootChallengerKey: '0xd42905d0582c476c4b74757be6576ec323d715a0c7dcff231b6348b7ab0190eb',
},
],
addresses: {
bootProposer: '0xfeEDA3882Dd44aeb394caEEf941386E7ed88e0E0',
bootChallenger: '0xfCb059A4dB5B961d3e48706fAC91a55Bad0035C9',
},
tokens: [
{
name: 'MockERC20',
address: '0xB5Acbe9a0F1F8B98F3fC04471F7fE5d2c222cB44',
amount: 200,
},
],
},
};
7 changes: 1 addition & 6 deletions docker-compose.ganache.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@ services:
image: trufflesuite/ganache-cli:v6.12.1
ports:
- 8546:8546
command:
ganache-cli --accounts=10 --defaultBalanceEther=1000 --gasLimit=0x3B9ACA00 --deterministic -i 4378921 -p 8546 -b 1
--account="0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69e,10000000000000000000000"
--account="0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69d,10000000000000000000000"
--account="0xd42905d0582c476c4b74757be6576ec323d715a0c7dcff231b6348b7ab0190eb,10000000000000000000000"
--account="0xfbc1ee1c7332e2e5a76a99956f50b3ba2639aff73d56477e877ef8390c41e0c6,10000000000000000000000"
command: ganache-cli --accounts=10 --defaultBalanceEther=1000 --gasLimit=0x3B9ACA00 --deterministic -i 4378921 -p 8546 -b 1 --account="0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69e,10000000000000000000000" --account="0x4775af73d6dc84a0ae76f8726bda4b9ecf187c377229cb39e1afa7a18236a69d,10000000000000000000000" --account="0xd42905d0582c476c4b74757be6576ec323d715a0c7dcff231b6348b7ab0190eb,10000000000000000000000" --account="0xfbc1ee1c7332e2e5a76a99956f50b3ba2639aff73d56477e877ef8390c41e0c6,10000000000000000000000"
signorecello marked this conversation as resolved.
Show resolved Hide resolved
networks:
- nightfall_network
20 changes: 10 additions & 10 deletions nightfall-deployer/contracts/Challenges.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract Challenges is Stateful, Key_Registry, Config {
uint256 blockNumberL2,
Transaction[] memory transactions,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
// check if the block hash is correct and the block hash exists for the block and prior block
state.isBlockReal(priorBlockL2, priorBlockTransactions, blockNumberL2 - 1);
Expand Down Expand Up @@ -73,7 +73,7 @@ contract Challenges is Stateful, Key_Registry, Config {
uint256 blockNumberL2,
Transaction[] memory transactions,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
// check if the block hash is correct and the block hash exists for the block and prior block
state.isBlockReal(priorBlockL2, priorBlockTransactions, blockNumberL2 - 1);
Expand Down Expand Up @@ -104,7 +104,7 @@ contract Challenges is Stateful, Key_Registry, Config {
uint256 transactionIndex1,
uint256 transactionIndex2,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
// first, check we have real, in-train, contiguous blocks
state.isBlockReal(block1, transactions1, block1NumberL2);
Expand Down Expand Up @@ -132,7 +132,7 @@ contract Challenges is Stateful, Key_Registry, Config {
Transaction[] memory transactions,
uint256 transactionIndex,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
state.isBlockReal(blockL2, transactions, blockNumberL2);
ChallengesUtil.libChallengeTransactionType(transactions[transactionIndex]);
Expand All @@ -148,7 +148,7 @@ contract Challenges is Stateful, Key_Registry, Config {
uint256 transactionIndex,
uint256[8] memory uncompressedProof,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
state.isBlockReal(blockL2, transactions, blockNumberL2);
// now we need to check that the proof is correct
Expand Down Expand Up @@ -176,7 +176,7 @@ contract Challenges is Stateful, Key_Registry, Config {
Transaction[] memory transactionsOfblockL2ContainingHistoricRoot,
uint256[8] memory uncompressedProof,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
state.isBlockReal(blockL2, transactions, blockNumberL2);
state.isBlockReal(
Expand Down Expand Up @@ -215,7 +215,7 @@ contract Challenges is Stateful, Key_Registry, Config {
Transaction[] memory transactionsOfblockL2ContainingHistoricRoot2,
uint256[8] memory uncompressedProof,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
state.isBlockReal(blockL2, transactions, blockNumberL2);
state.isBlockReal(
Expand Down Expand Up @@ -266,7 +266,7 @@ contract Challenges is Stateful, Key_Registry, Config {
uint256 transactionIndex2,
uint256 nullifierIndex2,
bytes32 salt
) public {
) public onlyBootChallenger {
checkCommit(msg.data);
ChallengesUtil.libChallengeNullifier(
txs1[transactionIndex1],
Expand Down Expand Up @@ -297,7 +297,7 @@ contract Challenges is Stateful, Key_Registry, Config {
Transaction[] memory transactions,
uint256 transactionIndex,
bytes32 salt
) external {
) external onlyBootChallenger {
checkCommit(msg.data);
state.isBlockReal(blockL2, transactions, blockNumberL2);
if (
Expand Down Expand Up @@ -367,7 +367,7 @@ contract Challenges is Stateful, Key_Registry, Config {
}

//To prevent frontrunning, we need to commit to a challenge before we send it
function commitToChallenge(bytes32 commitHash) external {
function commitToChallenge(bytes32 commitHash) external onlyBootChallenger {
require(committers[commitHash] == address(0), 'Hash is already committed to');
committers[commitHash] = msg.sender;
emit CommittedToChallenge(commitHash, msg.sender);
Expand Down
31 changes: 31 additions & 0 deletions nightfall-deployer/contracts/Config.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,43 @@ contract Config is Ownable {
uint256 constant COOLING_OFF_PERIOD = 1 weeks;
bytes32 constant ZERO = bytes32(0);

address bootProposer;
address bootChallenger;
mapping(address => uint256) erc20limit;

function initialize() public virtual override initializer {
Ownable.initialize();
}

// restricting proposers
modifier onlyBootProposer() {
require(msg.sender == bootProposer, 'You are not the boot proposer');
_;
}

function setBootProposer(address proposer) external onlyOwner {
Copy link
Contributor

@Westlad Westlad Mar 16, 2022

Choose a reason for hiding this comment

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

With this approach we can only have one challenger and one proposer address, unless I've misunderstood. Whilst, initially we want to restrict who can propose and who can challenge, I can image that quite early on we would want to add additional proposers and challengers, if only for resilience (e.g. to guard against a proposer running out of money or getting banned). We can of course upgrade the contract to make such a change but would it be good to start with a mapping of allowed proposers and challengers, so that the owner can add and remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I agree with you, but I think the next step will be to just remove all this, as we're definitely only gonna have one proposer and challenger. But I'll tag @ChaitanyaKonda to decide if it's worth to make it a mapping or if we can leave it as just one proposer and challenger

bootProposer = proposer;
}

function getBootProposer() external view returns (address) {
return bootProposer;
}

// restricting challengers
modifier onlyBootChallenger() {
require(msg.sender == bootChallenger, 'You are not the boot challenger');
_;
}

function setBootChallenger(address challenger) external onlyOwner {
bootChallenger = challenger;
}

function getBootChallenger() external view returns (address) {
return bootChallenger;
}

// restricting tokens
function getRestriction(address tokenAddr) public view returns (uint256) {
return erc20limit[tokenAddr];
}
Expand Down
2 changes: 1 addition & 1 deletion nightfall-deployer/contracts/Proposers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract Proposers is Stateful, Structures, Config, ReentrancyGuardUpgradeable {
}

//add the proposer to the circular linked list
function registerProposer() external payable nonReentrant {
function registerProposer() external payable nonReentrant onlyBootProposer {
require(REGISTRATION_BOND <= msg.value, 'The registration payment is incorrect');
require(
state.getProposer(msg.sender).thisAddress == address(0),
Expand Down
11 changes: 11 additions & 0 deletions nightfall-deployer/migrations/2_deploy_upgradeable.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ const Proposers = artifacts.require('Proposers.sol');
const Challenges = artifacts.require('Challenges.sol');
const State = artifacts.require('State.sol');

const config = require('config');

const { addresses } = config.RESTRICTIONS;

module.exports = async function (deployer) {
await deployer.deploy(Verifier);
await deployer.link(Verifier, [Challenges, ChallengesUtil]);
Expand All @@ -33,4 +37,11 @@ module.exports = async function (deployer) {
deployer,
unsafeAllowLinkedLibraries: true,
});

const proposers = await Proposers.deployed();
const challengers = await Challenges.deployed();
const { bootProposer, bootChallenger } = addresses;
console.log('ADDRESSES: ', addresses);
await proposers.setBootProposer(bootProposer);
await challengers.setBootChallenger(bootChallenger);
};
23 changes: 11 additions & 12 deletions test/e2e/protocol/challenger.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,29 @@ chai.use(chaiAsPromised);

const environment = config.ENVIRONMENTS[process.env.ENVIRONMENT] || config.ENVIRONMENTS.localhost;

const { mnemonics, signingKeys } = config.TEST_OPTIONS;
const { signingKeys } = config.RESTRICTIONS;
const { mnemonics } = config.TEST_OPTIONS;

const nf3Challenger = new Nf3(signingKeys.challenger, environment);
const bootChallenger = new Nf3(signingKeys.bootChallengerKey, environment);

describe('Basic Challenger tests', () => {
before(async () => {
await nf3Challenger.init(mnemonics.challenger);
// Challenger registration
await nf3Challenger.registerChallenger();
// Chalenger listening for incoming events
nf3Challenger.startChallenger();
await bootChallenger.init(mnemonics.challenger);
});

it('should register a challenger', async () => {
const res = await nf3Challenger.registerChallenger();
expect(res.status).to.be.equal(200);
it('should register the boot challenger', async () => {
signorecello marked this conversation as resolved.
Show resolved Hide resolved
// Challenger registration
await bootChallenger.registerChallenger();
// Chalenger listening for incoming events
bootChallenger.startChallenger();
});

it('should de-register a challenger', async () => {
const res = await nf3Challenger.deregisterChallenger();
const res = await bootChallenger.deregisterChallenger();
expect(res.status).to.be.equal(200);
});

after(async () => {
await nf3Challenger.close();
await bootChallenger.close();
});
});
Loading