Skip to content
Closed
Show file tree
Hide file tree
Changes from 18 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
9 changes: 8 additions & 1 deletion validator-cli/.env.dist
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
PRIVATE_KEY=

# Networks: devnet, testnet, mainnet
NETWORKS=devnet,testnet

# Devnet RPCs
RPC_CHIADO=https://rpc.chiadochain.net
RPC_ARB_SEPOLIA=https://sepolia-rollup.arbitrum.io/rpc
RPC_SEPOLIA=

# Devnet Owner
DEVNET_OWNER=0x5f4eC3Df9Cf2f0f1fDfCfCfCfCfCfCfCfCfCfCfC

# Testnet or Mainnet RPCs
RPC_ARB=https://sepolia-rollup.arbitrum.io/rpc
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 23, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove duplicate RPC URL configuration.

The Arbitrum RPC URL is defined twice:

  • Line 4: RPC_ARB_SEPOLIA=https://sepolia-rollup.arbitrum.io/rpc
  • Line 9: RPC_ARB=https://sepolia-rollup.arbitrum.io/rpc

This could lead to confusion. Consider removing one of them or clearly documenting the difference if both are needed.

Also applies to: 4-4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will use a single RPC var for both networks on a chain 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@mani99brar Great! Using a single RPC variable per chain is a good simplification that will make the configuration more maintainable. This is a change from your previous approach where you had separate variables for devnet and testnet/mainnet usage, but it should make the codebase cleaner and reduce potential confusion. Let me know if you need any help with implementing this change.


✏️ Learnings added
Learnt from: mani99brar
PR: kleros/vea#396
File: validator-cli/.env.dist:15-15
Timestamp: 2025-03-31T09:03:04.957Z
Learning: User (mani99brar) has decided to simplify the approach by using a single RPC variable per chain instead of maintaining separate variables for devnet and testnet/mainnet environments, even when they point to the same URL.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: madhurMongia
PR: kleros/vea#359
File: validator-cli/.env.dist:3-4
Timestamp: 2025-03-28T01:51:52.423Z
Learning: In 'validator-cli/.env.dist', the `RPC_GNOSIS` variable may be intentionally set to `https://rpc.chiadochain.net` as an example.

Learnt from: mani99brar
PR: kleros/vea#388
File: validator-cli/.env.dist:9-11
Timestamp: 2025-03-28T01:51:48.908Z
Learning: User (mani99brar) intentionally keeps the same RPC URLs under different environment variables to distinguish between devnet and testnet/mainnet usage, and this is by design rather than an oversight.

RPC_ETH=
Expand All @@ -20,7 +26,8 @@ VEAOUTBOX_ARB_TO_GNOSIS_ADDRESS=0x2f1788F7B74e01c4C85578748290467A5f063B0b
VEAROUTER_ARB_TO_GNOSIS_ADDRESS=0x5BE03fDE7794Bc188416ba16932510Ed1277b193
GNOSIS_AMB_ADDRESS=0x8448E15d0e706C0298dECA99F0b4744030e59d7d

VEAOUTBOX_CHAIN_ID=421611
VEAOUTBOX_CHAINS=11155111,421611
VEAOUTBOX_SUBGRAPH=https://api.studio.thegraph.com/query/user/outbox-arb-sep/version/latest

# Devnet Addresses
VEAINBOX_ARBSEPOLIA_TO_SEPOLIA_ADDRESS=0x906dE43dBef27639b1688Ac46532a16dc07Ce410
Expand Down
234 changes: 234 additions & 0 deletions validator-cli/src/ArbToEth/claimer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
import { ethers } from "ethers";
import { checkAndClaim, CheckAndClaimParams } from "./claimer";
import { ClaimHonestState } from "../utils/claim";
import { Network } from "../consts/bridgeRoutes";

describe("claimer", () => {
const NETWORK = Network.DEVNET;
let veaOutbox: any;
let veaInbox: any;
let veaInboxProvider: any;
let veaOutboxProvider: any;
let emitter: any;
let mockClaim: any;
let mockGetLatestClaimedEpoch: any;
let mockGetTransactionHandler: any;
let mockDeps: CheckAndClaimParams;

let mockTransactionHandler: any;
const mockTransactions = {
claimTxn: "0x111",
withdrawClaimDepositTxn: "0x222",
startVerificationTxn: "0x333",
verifySnapshotTxn: "0x444",
devnetAdvanceStateTxn: "0x555",
};
beforeEach(() => {
mockClaim = {
stateRoot: "0x1234",
claimer: "0xFa00D29d378EDC57AA1006946F0fc6230a5E3288",
timestampClaimed: 1234,
timestampVerification: 0,
blocknumberVerification: 0,
honest: 0,
challenger: ethers.ZeroAddress,
};
veaInbox = {
snapshots: jest.fn().mockResolvedValue(mockClaim.stateRoot),
};

veaOutbox = {
stateRoot: jest.fn().mockResolvedValue(mockClaim.stateRoot),
};
veaOutboxProvider = {
getBlock: jest.fn().mockResolvedValue({ number: 0, timestamp: 110 }),
};
emitter = {
emit: jest.fn(),
};

mockGetLatestClaimedEpoch = jest.fn();
mockGetTransactionHandler = jest.fn().mockReturnValue(function DummyTransactionHandler(params: any) {
// Return an object that matches our expected transaction handler.
return mockTransactionHandler;
});
mockDeps = {
chainId: 0,
claim: mockClaim,
network: NETWORK,
epoch: 10,
epochPeriod: 10,
veaInbox,
veaInboxProvider,
veaOutboxProvider,
veaOutbox,
transactionHandler: null,
emitter,
fetchLatestClaimedEpoch: mockGetLatestClaimedEpoch,
now: 110000, // (epoch+ 1) * epochPeriod * 1000 for claimable epoch
};

mockTransactionHandler = {
withdrawClaimDeposit: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.withdrawClaimDepositTxn = mockTransactions.withdrawClaimDepositTxn;
return Promise.resolve();
}),
makeClaim: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.claimTxn = mockTransactions.claimTxn;
return Promise.resolve();
}),
startVerification: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.startVerificationTxn = mockTransactions.startVerificationTxn;
return Promise.resolve();
}),
verifySnapshot: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.verifySnapshotTxn = mockTransactions.verifySnapshotTxn;
return Promise.resolve();
}),
transactions: {
claimTxn: "0x0",
withdrawClaimDepositTxn: "0x0",
startVerificationTxn: "0x0",
verifySnapshotTxn: "0x0",
},
};
});
afterEach(() => {
jest.clearAllMocks();
});
describe("checkAndClaim", () => {
beforeEach(() => {
mockTransactionHandler = {
withdrawClaimDeposit: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.withdrawClaimDepositTxn = mockTransactions.withdrawClaimDepositTxn;
return Promise.resolve();
}),
makeClaim: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.claimTxn = mockTransactions.claimTxn;
return Promise.resolve();
}),
startVerification: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.startVerificationTxn = mockTransactions.startVerificationTxn;
return Promise.resolve();
}),
verifySnapshot: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.verifySnapshotTxn = mockTransactions.verifySnapshotTxn;
return Promise.resolve();
}),
devnetAdvanceState: jest.fn().mockImplementation(() => {
mockTransactionHandler.transactions.devnetAdvanceStateTxn = mockTransactions.devnetAdvanceStateTxn;
return Promise.resolve();
}),
transactions: {
claimTxn: "0x0",
withdrawClaimDepositTxn: "0x0",
startVerificationTxn: "0x0",
verifySnapshotTxn: "0x0",
},
};
mockGetTransactionHandler = jest.fn().mockReturnValue(function DummyTransactionHandler(param: any) {
return mockTransactionHandler;
});
mockDeps.fetchTransactionHandler = mockGetTransactionHandler;
});
it("should return null if no claim is made for a passed epoch", async () => {
mockDeps.epoch = 7; // claimable epoch - 3
mockDeps.claim = null;

mockDeps.fetchTransactionHandler = mockGetTransactionHandler;
const result = await checkAndClaim(mockDeps);
expect(result).toBeNull();
});
it("should return null if no snapshot is saved on the inbox for a claimable epoch", async () => {
veaInbox.snapshots = jest.fn().mockResolvedValue(ethers.ZeroHash);
mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({
challenged: false,
stateroot: "0x1111",
});
mockDeps.claim = null;
mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch;
const result = await checkAndClaim(mockDeps);
expect(result).toBeNull();
});
it("should return null if there are no new messages in the inbox", async () => {
veaInbox.snapshots = jest.fn().mockResolvedValue(mockClaim.stateRoot);
mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({
challenged: false,
stateroot: "0x1111",
});
mockDeps.claim = null;
mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch;
const result = await checkAndClaim(mockDeps);
expect(result).toBeNull();
});
describe("devnet", () => {
beforeEach(() => {
mockDeps.network = Network.DEVNET;
});
it("should make a valid claim and advance state", async () => {
veaInbox.snapshots = jest.fn().mockResolvedValue("0x7890");
mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({
challenged: false,
stateroot: mockClaim.stateRoot,
});
mockDeps.transactionHandler = mockTransactionHandler;
mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch;
mockDeps.claim = null;
mockDeps.veaInbox = veaInbox;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.devnetAdvanceStateTxn).toBe(mockTransactions.devnetAdvanceStateTxn);
});
});
describe("testnet", () => {
beforeEach(() => {
mockDeps.network = Network.TESTNET;
});
it("should make a valid claim if no claim is made", async () => {
veaInbox.snapshots = jest.fn().mockResolvedValue("0x7890");
mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({
challenged: false,
stateroot: mockClaim.stateRoot,
});
mockDeps.transactionHandler = mockTransactionHandler;
mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch;
mockDeps.claim = null;
mockDeps.veaInbox = veaInbox;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.claimTxn).toBe(mockTransactions.claimTxn);
});
it("should make a valid claim if last claim was challenged", async () => {
veaInbox.snapshots = jest.fn().mockResolvedValue(mockClaim.stateRoot);
mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({
challenged: true,
stateroot: mockClaim.stateRoot,
});
mockDeps.transactionHandler = mockTransactionHandler;
mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch;
mockDeps.claim = null;
mockDeps.veaInbox = veaInbox;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.claimTxn).toEqual(mockTransactions.claimTxn);
});
it("should withdraw claim deposit if claimer is honest", async () => {
mockDeps.transactionHandler = mockTransactionHandler;
mockClaim.honest = ClaimHonestState.CLAIMER;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.withdrawClaimDepositTxn).toEqual(mockTransactions.withdrawClaimDepositTxn);
});
it("should start verification if verification is not started", async () => {
mockDeps.transactionHandler = mockTransactionHandler;
mockClaim.honest = ClaimHonestState.NONE;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.startVerificationTxn).toEqual(mockTransactions.startVerificationTxn);
});
it("should verify snapshot if verification is started", async () => {
mockDeps.transactionHandler = mockTransactionHandler;
mockClaim.honest = ClaimHonestState.NONE;
mockClaim.timestampVerification = 1234;
mockDeps.claim = mockClaim;
const result = await checkAndClaim(mockDeps);
expect(result.transactions.verifySnapshotTxn).toEqual(mockTransactions.verifySnapshotTxn);
});
});
});
});
116 changes: 116 additions & 0 deletions validator-cli/src/ArbToEth/claimer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { EventEmitter } from "events";
import { ethers } from "ethers";
import { JsonRpcProvider } from "@ethersproject/providers";
import { getClaim, ClaimHonestState } from "../utils/claim";
import { getLastClaimedEpoch } from "../utils/graphQueries";
import { ArbToEthTransactionHandler } from "./transactionHandler";
import { BotEvents } from "../utils/botEvents";
import { ClaimStruct } from "@kleros/vea-contracts/typechain-types/arbitrumToEth/VeaInboxArbToEth";
import { ArbToEthDevnetTransactionHandler } from "./transactionHandlerDevnet";
import { getTransactionHandler } from "../utils/ethers";
import { Network } from "../consts/bridgeRoutes";
interface CheckAndClaimParams {
chainId: number;
network: Network;
claim: ClaimStruct | null;
epochPeriod: number;
epoch: number;
veaInbox: any;
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 23, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace 'any' with specific contract types for 'veaInbox' and 'veaOutbox'

Using any for veaInbox and veaOutbox defeats the purpose of TypeScript's type safety. Replace any with the specific contract types to improve type checking and code maintainability. This change will help catch errors at compile time and provide better IntelliSense support.

Apply this diff to fix the issue:

 interface checkAndClaimParams {
   epochPeriod: number;
   epoch: number;
-  veaInbox: any;
+  veaInbox: VeaInboxArbToEth;
   veaInboxProvider: JsonRpcProvider;
-  veaOutbox: any;
+  veaOutbox: VeaOutboxArbToEth;
   veaOutboxProvider: JsonRpcProvider;
   transactionHandler: ArbToEthTransactionHandler | null;
   emitter: EventEmitter;

Also applies to: 13-13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

watcher.ts is chain agnostic and has different claimers, so to support it any type was used.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mani99brar I understand your reasoning. Using any type makes sense here for maintaining chain agnosticism since different chains have their own specific contract types. This is consistent with the existing pattern in the codebase that I've seen before. Thanks for the clarification!


🧠 Learnings used
Learnt from: mani99brar
PR: kleros/vea#388
File: validator-cli/src/ArbToEth/validator.ts:16-17
Timestamp: 2025-03-28T01:51:35.847Z
Learning: In the validator-cli, `any` type is intentionally used for `veaInbox` and `veaOutbox` parameters to maintain chain agnosticism. This is because different chains have their own specific contract types (e.g., VeaInboxArbToEth, VeaOutboxArbToEth, VeaOutboxArbToGnosis) and using union types would make the code more complex and less maintainable.

Learnt from: mani99brar
PR: kleros/vea#388
File: validator-cli/src/ArbToEth/validator.ts:16-17
Timestamp: 2025-03-28T01:51:48.908Z
Learning: In the validator-cli, `any` type is intentionally used for `veaInbox` and `veaOutbox` parameters to maintain chain agnosticism, as different chains have their own contract types.

veaInboxProvider: JsonRpcProvider;
veaOutbox: any;
veaOutboxProvider: JsonRpcProvider;
transactionHandler: ArbToEthTransactionHandler | null;
emitter: EventEmitter;
fetchClaim?: typeof getClaim;
fetchLatestClaimedEpoch?: typeof getLastClaimedEpoch;
fetchTransactionHandler?: typeof getTransactionHandler;
now?: number;
}

async function checkAndClaim({
chainId,
network,
claim,
epoch,
epochPeriod,
veaInbox,
veaInboxProvider,
veaOutbox,
veaOutboxProvider,
transactionHandler,
emitter,
fetchLatestClaimedEpoch = getLastClaimedEpoch,
fetchTransactionHandler = getTransactionHandler,
now = Date.now(),
}: CheckAndClaimParams) {
let outboxStateRoot = await veaOutbox.stateRoot();
const finalizedOutboxBlock = await veaOutboxProvider.getBlock("finalized");
const claimAbleEpoch = Math.floor(now / (1000 * epochPeriod)) - 1;
if (!transactionHandler) {
const TransactionHandler = fetchTransactionHandler(chainId, network);
transactionHandler = new TransactionHandler({
network,
epoch,
veaInbox,
veaOutbox,
veaInboxProvider,
veaOutboxProvider,
emitter,
claim,
});
} else {
transactionHandler.claim = claim;
}
var savedSnapshot;
var claimData;
var newMessagesToBridge: boolean;
var lastClaimChallenged: boolean;

if (network == Network.DEVNET) {
const devnetTransactionHandler = transactionHandler as ArbToEthDevnetTransactionHandler;
if (claim == null) {
[savedSnapshot, claimData] = await Promise.all([
veaInbox.snapshots(epoch),
fetchLatestClaimedEpoch(veaOutbox.target),
]);

newMessagesToBridge = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.ZeroHash;
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 31, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace ethers.ZeroHash & ethers.ZeroAddress with standardized constants.
ethers.ZeroHash and ethers.ZeroAddress may not exist in certain Ethers versions. The standard constants are ethers.constants.HashZero and ethers.constants.AddressZero.

Apply this diff to fix the references:

- newMessagesToBridge = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.ZeroHash;
+ newMessagesToBridge = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.constants.HashZero;

- if (claim.challenger != ethers.ZeroAddress) {
+ if (claim.challenger != ethers.constants.AddressZero) {

Also applies to: 99-99

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 constants are correct for "ethers v6"

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@mani99brar You're absolutely correct. In ethers.js v6, the constants were reorganized and ethers.ZeroHash and ethers.ZeroAddress are indeed the proper way to reference these values, replacing the v5 approach of ethers.constants.HashZero and ethers.constants.AddressZero. Thank you for the clarification.


🌐 Web query:

What are the correct constants for zero hash and zero address in ethers.js v6?

💡 Result:

In Ethers.js v6, the constants for zero hash and zero address have been updated from their v5 counterparts. Here are the correct constants:

Zero Address
ethers.ZeroAddress
Represents the Ethereum zero address as a string:
"0x0000000000000000000000000000000000000000" [6][7]

Zero Hash
ethers.ZeroHash
Represents the 32-byte zero hash as a string:
"0x0000000000000000000000000000000000000000000000000000000000000000" [6][7]

Key changes from v5 to v6:

  • Removed nested constants object
  • Renamed AddressZeroZeroAddress
  • Renamed HashZeroZeroHash
  • Now uses direct top-level exports instead of properties under ethers.constants [8][12]

These constants are commonly used for:

  1. Initializing empty address fields in smart contracts
  2. Checking for unset values
  3. Default parameters in transaction configurations
  4. Hash comparisons in cryptographic operations [1][6][13]

Example usage:

import { ZeroAddress, ZeroHash } from 'ethers';

console.log(ZeroAddress); // 0x0000...0000
console.log(ZeroHash);    // 0x0000...0000

Citations:


@mani99brar You're absolutely correct. In ethers.js v6, the constants were reorganized and ethers.ZeroHash and ethers.ZeroAddress are indeed the proper way to reference these values, replacing the v5 approach of ethers.constants.HashZero and ethers.constants.AddressZero. Thank you for the clarification.


✏️ Learnings added
Learnt from: mani99brar
PR: kleros/vea#396
File: validator-cli/src/ArbToEth/claimer.ts:77-77
Timestamp: 2025-03-31T09:03:43.595Z
Learning: In ethers.js v6, the correct constants for zero values are `ethers.ZeroHash` and `ethers.ZeroAddress`, which replaced the v5 constants `ethers.constants.HashZero` and `ethers.constants.AddressZero`.

Learnt from: mani99brar
PR: kleros/vea#396
File: validator-cli/src/ArbToEth/claimer.ts:77-77
Timestamp: 2025-03-31T09:03:43.595Z
Learning: In ethers.js v6, the correct constants for zero values are `ethers.ZeroHash` and `ethers.ZeroAddress`, which replaced the v5 constants `ethers.constants.HashZero` and `ethers.constants.AddressZero`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

if (newMessagesToBridge && savedSnapshot != ethers.ZeroHash) {
await devnetTransactionHandler.devnetAdvanceState(savedSnapshot);
return devnetTransactionHandler;
}
}
} else if (claim == null && epoch == claimAbleEpoch) {
[savedSnapshot, claimData] = await Promise.all([
veaInbox.snapshots(epoch),
fetchLatestClaimedEpoch(veaOutbox.target),
]);
newMessagesToBridge = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.ZeroHash;
lastClaimChallenged = claimData.challenged && savedSnapshot == outboxStateRoot;
if ((newMessagesToBridge || lastClaimChallenged) && savedSnapshot != ethers.ZeroHash) {
await transactionHandler.makeClaim(savedSnapshot);
return transactionHandler;
}
} else if (claim != null) {
if (claim.honest == ClaimHonestState.CLAIMER) {
await transactionHandler.withdrawClaimDeposit();
return transactionHandler;
} else if (claim.honest == ClaimHonestState.NONE) {
if (claim.challenger != ethers.ZeroAddress) {
emitter.emit(BotEvents.CLAIM_CHALLENGED, epoch);
return transactionHandler;
}
if (claim.timestampVerification == 0) {
await transactionHandler.startVerification(finalizedOutboxBlock.timestamp);
} else {
await transactionHandler.verifySnapshot(finalizedOutboxBlock.timestamp);
}
return transactionHandler;
}
} else {
emitter.emit(BotEvents.CLAIM_EPOCH_PASSED, epoch);
}
return null;
}

export { checkAndClaim, CheckAndClaimParams };
Loading