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/bridger-cli #370

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from
15 changes: 15 additions & 0 deletions bridger-cli/.env.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
PRIVATE_KEY=


VEAOUTBOX_CHAIN_ID=11155111

# VeaInbox Address and Rpc Provider
VEAINBOX_ADDRESS=0xE12daFE59Bc3A996362d54b37DFd2BA9279cAd06
VEAINBOX_PROVIDER=http://localhost:8545

# VeaOutbox Address and Rpc Provider
VEAOUTBOX_ADDRESS=0x209BFdC6B7c66b63A8382196Ba3d06619d0F12c9
VEAOUTBOX_PROVIDER=http://localhost:8546

# Ex: https://api.studio.thegraph.com/query/85918/outbox-arb-sep-sep-testnet-vs/version/latest
VEAOUTBOX_SUBGRAPH=http://localhost:8000/subgraphs/name/kleros/veascan-outbox/graphql
10 changes: 10 additions & 0 deletions bridger-cli/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { Config } from "jest";

const config: Config = {
preset: "ts-jest",
testEnvironment: "node",
collectCoverage: true,
collectCoverageFrom: ["**/*.ts"],
};

export default config;
28 changes: 28 additions & 0 deletions bridger-cli/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"name": "@kleros/bridger-cli",
"license": "MIT",
"packageManager": "yarn@4.2.2",
"engines": {
"node": ">=18.0.0"
},
"volta": {
"node": "18.20.3",
"yarn": "4.2.2"
},
"scripts": {
"start-bridger": "npx ts-node ./src/bridger.ts",
"test": "jest --coverage"
},
"dependencies": {
"@kleros/vea-contracts": "workspace:^",
"@typechain/ethers-v5": "^10.2.0",
"dotenv": "^16.4.5",
"typescript": "^4.9.5",
"web3": "^1.10.4"
},
"devDependencies": {
"@types/jest": "^29.5.14",
"jest": "^29.7.0",
"ts-jest": "^29.2.5"
}
}
321 changes: 321 additions & 0 deletions bridger-cli/src/bridger.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Brilliantly resolved. I left a few comments but I am utterly happy with the job you did

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for all the help 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a type mismatch issue for tests which include value : deposit used for mocking, the bot itself works. I wasn't able to solve it for now will raise an issue and fix later.

Original file line number Diff line number Diff line change
@@ -0,0 +1,321 @@
require("dotenv").config();
import { assert } from "chai";
import { hashClaim } from "./utils/claim";
import { getVeaOutbox, getVeaInbox } from "./utils/ethers";
import { watch } from "./bridger";
import { ShutdownSignal } from "./utils/shutdown";
import { JsonRpcProvider } from "@ethersproject/providers";
import { BigNumber } from "ethers";

jest.setTimeout(15000);
describe("bridger", function () {
console.log = function () {};
const ZERO_HASH = "0x0000000000000000000000000000000000000000000000000000000000000000";
const FAKE_HASH = "0x0000000000000000000000000000000000000000000000000000000000000001";
const mockMessage = {
data: "0x000000000000000000000000abcdefabcdefabcdefabcdefabcdefabcd00000000000000000000000000000000000000000000000000000000000003e8",
to: "0x1234567890abcdef1234567890abcdef12345678",
fnSelector: "0x12345678",
};
const inboxProvider = new JsonRpcProvider("http://localhost:8545");
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a good candidate for env var. I know the value will be probably fixed, but if you need to change the port at some point you can do it easily from the .env file without having to edit multiple individual files.

The env var also provides a name for the value, so instead of using the raw URLs (that are very similar) you use the HARDHAT_INBOX_RPC (for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hardcoded it for now, because I planned to do it when we test for more routes. Ideally we want to pass env into docker compose file with networks and rpc ports. Also the graph node needs the rpc port for indexing. I ll do it in a future PR when I setup testing environment for relayer or validator.

const outboxProvider = new JsonRpcProvider("http://localhost:8546");

let claimEpoch: number;
let epochPeriod: number;
let deposit: BigNumber;
let sequencerDelay: number;

const veaInbox = getVeaInbox(
process.env.VEAINBOX_ADDRESS,
process.env.PRIVATE_KEY,
"http://localhost:8545",
Number(process.env.VEAOUTBOX_CHAIN_ID)
);
const veaOutbox = getVeaOutbox(
process.env.VEAOUTBOX_ADDRESS,
process.env.PRIVATE_KEY,
"http://localhost:8546",
Number(process.env.VEAOUTBOX_CHAIN_ID)
);

// Increase epoch on both evn chains to maintain consistency
async function increaseEpoch() {
await inboxProvider.send("evm_increaseTime", [epochPeriod]);
await outboxProvider.send("evm_increaseTime", [epochPeriod]);
await inboxProvider.send("evm_mine", []);
await outboxProvider.send("evm_mine", []);
}

// Start bridger with a timeout
async function startBridgerWithTimeout(timeout: number, startEpoch: number = 0) {
const shutDownSignal = new ShutdownSignal();
const bridgerPromise = watch(shutDownSignal, startEpoch);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine and I am happy that you managed to figure out a way. Honestly, congrats on that because this kind of code is not always easy to test.

I'll try to provide an refactor suggestion on watch that may make the code easier to test. I'll leave the code when I arrive to that section (hope I don't forget! :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup this was a bit tricky, also it doesn't work really good. Sometimes the tests end and the watch doesn't end, wasn't able to resolve it.

const timeoutPromise = new Promise((resolve) => {
setTimeout(() => {
shutDownSignal.setShutdownSignal();
resolve("Timeout reached");
}, timeout);
});
await Promise.race([bridgerPromise, timeoutPromise]);
}

// Sync epochs on both chains
async function syncEpochs() {
const inboxEpoch = Number(await veaInbox.epochNow());
const outboxEpoch = Number(await veaOutbox.epochNow());
if (inboxEpoch !== outboxEpoch) {
if (inboxEpoch > outboxEpoch) {
await outboxProvider.send("evm_increaseTime", [epochPeriod * (inboxEpoch - outboxEpoch)]);
await outboxProvider.send("evm_mine", []);
} else {
await inboxProvider.send("evm_increaseTime", [epochPeriod * (outboxEpoch - inboxEpoch)]);
await inboxProvider.send("evm_mine", []);
}
}
}

beforeEach(async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

(ignoreable) I have not deeply inspected, but some of the async behavior look independent from the next line and can probably be called with Promise.all to make tests run faster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup I need to incorporate Promise.all in my code more, will make the changes.

epochPeriod = Number(await veaOutbox.epochPeriod());
deposit = await veaOutbox.deposit();
await increaseEpoch();
await syncEpochs();
claimEpoch = Number(await veaInbox.epochNow());
sequencerDelay = Number(await veaOutbox.sequencerDelayLimit());
});

describe("Integration tests: Claiming", function () {
it("should claim for new saved snapshot", async function () {
// Send a message and save snapshot
await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();

// Increase epoch so that claim can be made
await increaseEpoch();

// Start bridger
await startBridgerWithTimeout(1000, claimEpoch);

const toBeClaimedStateRoot = await veaInbox.snapshots(claimEpoch);
const claimData = await veaOutbox.queryFilter(veaOutbox.filters.Claimed(null, claimEpoch, null));
const bridger = `0x${claimData[0].topics[1].slice(26)}`;
const claimBlock = await outboxProvider.getBlock(claimData[0].blockNumber);
const claim = {
stateRoot: toBeClaimedStateRoot,
claimer: bridger,
timestampClaimed: claimBlock.timestamp,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: "0x0000000000000000000000000000000000000000",
};
// Check if claim was made
const claimHash = await veaOutbox.claimHashes(claimEpoch);
assert.notEqual(claimHash, ZERO_HASH, "Claim was not made");
assert.equal(claimHash, hashClaim(claim), "Wrong claim was made");
});

it("should not claim for old snapshot", async function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

(ignoreable) usually it is best if a test do not depend on state from a previous test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test should not be dependent on the previous test, have I missed something? I replicated the environment in the test and it should be independent of any test.

await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);

// Increase epoch on both evn chains to maintain consistency
await increaseEpoch();

// Start bridger
await startBridgerWithTimeout(5000, claimEpoch);

// Assert no new claims were made since last claim
const currentEpochClaim = await veaOutbox.claimHashes(claimEpoch);
assert.equal(currentEpochClaim, ZERO_HASH, "Claim was made");
});

it("should make new claim if last claim was challenged", async function () {
await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();

// Increase epoch on both evn chains to maintain consistency
await increaseEpoch();
await veaInbox.saveSnapshot();

// Make claim and challenge it
const claimTxn = await veaOutbox.claim(claimEpoch, FAKE_HASH, { value: deposit });
const claimBlock = await outboxProvider.getBlock(claimTxn.blockNumber);

const claim = {
stateRoot: FAKE_HASH,
claimer: claimTxn.from,
timestampClaimed: claimBlock.timestamp,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: "0x0000000000000000000000000000000000000000",
};
await veaOutbox["challenge(uint256,(bytes32,address,uint32,uint32,uint32,uint8,address))"](claimEpoch, claim, {
value: deposit,
});
// Increase epoch
await increaseEpoch();

await startBridgerWithTimeout(5000, claimEpoch);

// Check if claim was made
const claimLogs = await veaOutbox.queryFilter(veaOutbox.filters.Claimed(null, claimEpoch + 1, null));

const snapshotOnInbox = await veaInbox.snapshots(claimEpoch + 1);
assert.equal(claimLogs.length, 1, "Claim was not made");
assert.equal(Number(claimLogs[0].topics[2]), claimEpoch + 1, "Claim was made for wrong epoch");
assert.equal(snapshotOnInbox, claimLogs[0].data, "Snapshot was not saved");
});
});

describe("Integration tests: Verification", function () {
it("should start verification when claim is verifiable", async function () {
await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();
await increaseEpoch();

const savedSnapshot = await veaInbox.snapshots(claimEpoch);
await veaOutbox.claim(claimEpoch, savedSnapshot, { value: deposit });
await outboxProvider.send("evm_mine", []);

await inboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await inboxProvider.send("evm_mine", []);
await outboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await outboxProvider.send("evm_mine", []);

await startBridgerWithTimeout(5000, claimEpoch);

// Check if verification was started
const verificationLogs = await veaOutbox.queryFilter(veaOutbox.filters.VerificationStarted(claimEpoch));
assert.equal(verificationLogs.length, 1, "Verification was not started");
});

it("should not verify claim when claim is challenged", async function () {
await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();

await increaseEpoch();

// Make claim and challenge it
const claimTx = await veaOutbox.claim(claimEpoch, FAKE_HASH, { value: deposit });

const oldClaimLogs = await veaOutbox.queryFilter(veaOutbox.filters.Claimed(null, claimEpoch, null));
const claimBlock = await outboxProvider.getBlock(oldClaimLogs[0].blockNumber);

let claim = {
stateRoot: FAKE_HASH,
claimer: claimTx.from,
timestampClaimed: claimBlock.timestamp,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: "0x0000000000000000000000000000000000000000",
};
await veaOutbox["challenge(uint256,(bytes32,address,uint32,uint32,uint32,uint8,address))"](claimEpoch, claim, {
value: deposit,
});
await outboxProvider.send("evm_mine", []);

await inboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await inboxProvider.send("evm_mine", []);
await outboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await outboxProvider.send("evm_mine", []);

await startBridgerWithTimeout(5000, claimEpoch);

const verificationLogs = await veaOutbox.queryFilter(veaOutbox.filters.VerificationStarted(claimEpoch));
assert.equal(verificationLogs.length, 0, "Verification was started");
});

it("should verify snapshot when claim is verified", async function () {
// Also add a test for the case when the claim is not verifiable

await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();
await increaseEpoch();

const claimTxn = await veaOutbox.claim(claimEpoch, FAKE_HASH, { value: deposit });
const claimBlock = await outboxProvider.getBlock(claimTxn.blockNumber);
await inboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await inboxProvider.send("evm_mine", []);
await outboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await outboxProvider.send("evm_mine", []);

var claim = {
stateRoot: FAKE_HASH,
claimer: claimTxn.from,
timestampClaimed: claimBlock.timestamp,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: "0x0000000000000000000000000000000000000000",
};
const verifyTxn = await veaOutbox.startVerification(claimEpoch, claim);
const verifyBlock = await outboxProvider.getBlock(verifyTxn.blockNumber);
claim.timestampVerification = verifyBlock.timestamp;
claim.blocknumberVerification = verifyBlock.number;

const minChallengePeriod = Number(await veaOutbox.minChallengePeriod());

await outboxProvider.send("evm_increaseTime", [minChallengePeriod]);
await outboxProvider.send("evm_mine", []);

await startBridgerWithTimeout(5000, claimEpoch);

const postVerifyStateRoot = await veaOutbox.stateRoot();
const latestVerifiedEpoch = await veaOutbox.latestVerifiedEpoch();

assert.equal(postVerifyStateRoot, FAKE_HASH, "Snapshot was not verified");
assert.equal(Number(latestVerifiedEpoch), claimEpoch, "Snapshot was veified for wrong epoch");
});

it("should withdraw deposit when claimer is honest", async function () {
await veaInbox.sendMessage(mockMessage.to, mockMessage.fnSelector, mockMessage.data);
await veaInbox.saveSnapshot();
await increaseEpoch();

const claimTxn = await veaOutbox.claim(claimEpoch, FAKE_HASH, { value: deposit });
const claimBlock = await outboxProvider.getBlock(claimTxn.blockNumber);
await inboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await inboxProvider.send("evm_mine", []);
await outboxProvider.send("evm_increaseTime", [epochPeriod + sequencerDelay]);
await outboxProvider.send("evm_mine", []);

var claim = {
stateRoot: FAKE_HASH,
claimer: claimTxn.from,
timestampClaimed: claimBlock.timestamp,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: "0x0000000000000000000000000000000000000000",
};
const verifyTxn = await veaOutbox.startVerification(claimEpoch, claim);
const verifyBlock = await outboxProvider.getBlock(verifyTxn.blockNumber);
claim.timestampVerification = verifyBlock.timestamp;
claim.blocknumberVerification = verifyBlock.number;

const minChallengePeriod = Number(await veaOutbox.minChallengePeriod());

await outboxProvider.send("evm_increaseTime", [minChallengePeriod]);
await outboxProvider.send("evm_mine", []);

await veaOutbox.verifySnapshot(claimEpoch, claim);

const balancePreWithdraw = Number(await outboxProvider.getBalance(claimTxn.from));
const contractBalancePreWithdraw = Number(await outboxProvider.getBalance(veaOutbox.address));

await startBridgerWithTimeout(5000, claimEpoch);
const balancePostWithdraw = Number(await outboxProvider.getBalance(claimTxn.from));
const contractBalancePostWithdraw = Number(await outboxProvider.getBalance(veaOutbox.address));
// Check if deposit was withdrawn
assert.isAbove(balancePostWithdraw, balancePreWithdraw, "Deposit was not withdrawn");
assert.equal(
contractBalancePostWithdraw,
contractBalancePreWithdraw - Number(deposit),
"Deposit was not withdrawn"
);
mani99brar marked this conversation as resolved.
Show resolved Hide resolved
});

it.todo("should not withdraw deposit when claimer is dishonest");
});
});
Loading
Loading