-
Notifications
You must be signed in to change notification settings - Fork 7
Feat/happy path(claimer) in validator-cli #396
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
Changes from 5 commits
ad77cf4
371e8d0
7173b75
240f5f7
a25b473
cfa2e02
105a50a
edce69c
3f78877
4eb9b3c
06220dd
84ab53f
5b0740a
2e67ae2
af153e1
2316c21
88deca7
bedb92c
d85dd2d
3b3d905
f1bb940
54fbed2
66c1fc6
410a068
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| import { ethers } from "ethers"; | ||
| import { checkAndClaim } from "./claimer"; | ||
| import { ArbToEthTransactionHandler } from "./transactionHandler"; | ||
| import { ClaimHonestState } from "../utils/claim"; | ||
| import { start } from "pm2"; | ||
| describe("claimer", () => { | ||
| let veaOutbox: any; | ||
| let veaInbox: any; | ||
| let veaInboxProvider: any; | ||
| let veaOutboxProvider: any; | ||
| let emitter: any; | ||
| let mockGetClaim: any; | ||
| let mockClaim: any; | ||
| let mockGetLatestClaimedEpoch: any; | ||
| let mockDeps: any; | ||
| 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: 100 }), | ||
| }; | ||
| emitter = { | ||
| emit: jest.fn(), | ||
| }; | ||
|
|
||
| mockGetClaim = jest.fn(); | ||
| mockGetLatestClaimedEpoch = jest.fn(); | ||
| mockDeps = { | ||
| epoch: 10, | ||
| epochPeriod: 10, | ||
| veaInbox, | ||
| veaInboxProvider, | ||
| veaOutboxProvider, | ||
| veaOutbox, | ||
| transactionHandler: null, | ||
| emitter, | ||
| fetchClaim: mockGetClaim, | ||
| fetchLatestClaimedEpoch: mockGetLatestClaimedEpoch, | ||
| }; | ||
| }); | ||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
| describe("checkAndClaim", () => { | ||
| let mockTransactionHandler: any; | ||
| const mockTransactions = { | ||
| claimTxn: "0x111", | ||
| withdrawClaimDepositTxn: "0x222", | ||
| startVerificationTxn: "0x333", | ||
| verifySnapshotTxn: "0x444", | ||
| }; | ||
| 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(); | ||
| }), | ||
| transactions: { | ||
| claimTxn: "0x0", | ||
| withdrawClaimDepositTxn: "0x0", | ||
| startVerificationTxn: "0x0", | ||
| verifySnapshotTxn: "0x0", | ||
| }, | ||
| }; | ||
| }); | ||
| it("should return null if no claim is made for a passed epoch", async () => { | ||
| mockGetClaim = jest.fn().mockReturnValue(null); | ||
| mockDeps.epoch = 7; // claimable epoch - 3 | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| 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 () => { | ||
| mockGetClaim = jest.fn().mockReturnValue(null); | ||
| veaInbox.snapshots = jest.fn().mockResolvedValue(ethers.ZeroHash); | ||
| mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({ | ||
| challenged: false, | ||
| stateroot: "0x1111", | ||
| }); | ||
| 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 () => { | ||
| mockGetClaim = jest.fn().mockReturnValue(null); | ||
| veaInbox.snapshots = jest.fn().mockResolvedValue(mockClaim.stateRoot); | ||
| mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({ | ||
| challenged: false, | ||
| stateroot: "0x1111", | ||
| }); | ||
| mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch; | ||
| const result = await checkAndClaim(mockDeps); | ||
| expect(result).toBeNull(); | ||
| }); | ||
| it("should make a valid calim if no claim is made", async () => { | ||
| mockGetClaim = jest.fn().mockReturnValue(null); | ||
| veaInbox.snapshots = jest.fn().mockResolvedValue("0x7890"); | ||
| mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({ | ||
| challenged: false, | ||
| stateroot: mockClaim.stateRoot, | ||
| }); | ||
| mockDeps.transactionHandler = mockTransactionHandler; | ||
| mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch; | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| mockDeps.veaInbox = veaInbox; | ||
| const result = await checkAndClaim(mockDeps); | ||
| expect(result.transactions.claimTxn).toBe(mockTransactions.claimTxn); | ||
| }); | ||
| it("should make a valid calim if last claim was challenged", async () => { | ||
| mockGetClaim = jest.fn().mockReturnValue(null); | ||
| veaInbox.snapshots = jest.fn().mockResolvedValue(mockClaim.stateRoot); | ||
| mockGetLatestClaimedEpoch = jest.fn().mockResolvedValue({ | ||
| challenged: true, | ||
| stateroot: mockClaim.stateRoot, | ||
| }); | ||
| mockDeps.transactionHandler = mockTransactionHandler; | ||
| mockDeps.fetchLatestClaimedEpoch = mockGetLatestClaimedEpoch; | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| 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; | ||
|
|
||
| mockGetClaim = jest.fn().mockResolvedValue(mockClaim); | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| 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; | ||
| mockGetClaim = jest.fn().mockResolvedValue(mockClaim); | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| 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; | ||
| mockGetClaim = jest.fn().mockResolvedValue(mockClaim); | ||
| mockDeps.fetchClaim = mockGetClaim; | ||
| const result = await checkAndClaim(mockDeps); | ||
| expect(result.transactions.verifySnapshotTxn).toEqual(mockTransactions.verifySnapshotTxn); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||
| 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"; | ||||||
| interface checkAndClaimParams { | ||||||
| epochPeriod: number; | ||||||
| epoch: number; | ||||||
| veaInbox: any; | ||||||
| veaInboxProvider: JsonRpcProvider; | ||||||
| veaOutbox: any; | ||||||
| veaOutboxProvider: JsonRpcProvider; | ||||||
| transactionHandler: ArbToEthTransactionHandler | null; | ||||||
| emitter: EventEmitter; | ||||||
| fetchClaim?: typeof getClaim; | ||||||
| fetchLatestClaimedEpoch?: typeof getLastClaimedEpoch; | ||||||
| } | ||||||
|
|
||||||
| export async function checkAndClaim({ | ||||||
| epoch, | ||||||
| epochPeriod, | ||||||
| veaInbox, | ||||||
| veaInboxProvider, | ||||||
| veaOutbox, | ||||||
| veaOutboxProvider, | ||||||
| transactionHandler, | ||||||
| emitter, | ||||||
| fetchClaim = getClaim, | ||||||
| fetchLatestClaimedEpoch = getLastClaimedEpoch, | ||||||
| }: checkAndClaimParams) { | ||||||
| let outboxStateRoot = await veaOutbox.stateRoot(); | ||||||
| const finalizedOutboxBlock = await veaOutboxProvider.getBlock("finalized"); | ||||||
| const claimAbleEpoch = finalizedOutboxBlock.timestamp / epochPeriod; | ||||||
|
||||||
| const claimAbleEpoch = finalizedOutboxBlock.timestamp / epochPeriod; | |
| const claimableEpoch = Math.floor(finalizedOutboxBlock.timestamp / epochPeriod); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the use of 'ethers.ZeroHash' to 'ethers.constants.HashZero'
The property ethers.ZeroHash does not exist. The correct constant for the zero hash in ethers.js is ethers.constants.HashZero. Update the code to use the correct constant.
Apply this diff to fix the issue:
- const newMessagesToBridge: boolean = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.ZeroHash;
+ const newMessagesToBridge: boolean = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.constants.HashZero;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const newMessagesToBridge: boolean = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.ZeroHash; | |
| const newMessagesToBridge: boolean = savedSnapshot != outboxStateRoot && savedSnapshot != ethers.constants.HashZero; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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
anyforveaInboxandveaOutboxdefeats the purpose of TypeScript's type safety. Replaceanywith 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
watcher.tsis chain agnostic and has different claimers, so to support itanytype was used.There was a problem hiding this comment.
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
anytype 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