-
Notifications
You must be signed in to change notification settings - Fork 0
Factory update: Agoric validation, nonce tracking, no payload/gas #20
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
base: main
Are you sure you want to change the base?
Changes from all commits
addf6c3
6a2fb63
32b5313
dd645c5
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 |
---|---|---|
|
@@ -32,7 +32,7 @@ describe("Factory", () => { | |
const abiCoder = new ethers.AbiCoder(); | ||
const expectedWalletAddress = "0x856e4424f806D16E8CBC702B3c0F2ede5468eae5"; | ||
|
||
const sourceContract = "agoric"; | ||
const sourceChain = "agoric"; | ||
const sourceAddress = "0x1234567890123456789012345678901234567890"; | ||
|
||
let commandIdCounter = 1; | ||
|
@@ -75,7 +75,6 @@ describe("Factory", () => { | |
factory = await Contract.deploy( | ||
axelarGatewayMock.target, | ||
axelarGasServiceMock.target, | ||
"Ethereum", | ||
); | ||
await factory.waitForDeployment(); | ||
|
||
|
@@ -93,7 +92,7 @@ describe("Factory", () => { | |
}); | ||
}); | ||
|
||
it("fund Factory with ETH to pay for gas", async () => { | ||
it("fund Factory with ETH", async () => { | ||
const provider = ethers.provider; | ||
|
||
const factoryAddress = await factory.getAddress(); | ||
|
@@ -116,7 +115,7 @@ describe("Factory", () => { | |
return null; | ||
} | ||
}) | ||
.find((parsed) => parsed && parsed.name === "Received"); | ||
.find((parsed) => parsed && parsed.name === "TokensReceived"); | ||
|
||
expect(receivedEvent).to.not.be.undefined; | ||
expect(receivedEvent?.args.sender).to.equal(owner.address); | ||
|
@@ -129,12 +128,13 @@ describe("Factory", () => { | |
it("should create a new remote wallet using Factory", async () => { | ||
const commandId = getCommandId(); | ||
|
||
const payload = abiCoder.encode(["uint256"], [50000]); | ||
const nonce = 1; | ||
const payload = abiCoder.encode(["uint256"], [nonce]); | ||
const payloadHash = keccak256(toBytes(payload)); | ||
|
||
await approveMessage({ | ||
commandId, | ||
from: sourceContract, | ||
from: sourceChain, | ||
sourceAddress, | ||
targetAddress: factory.target, | ||
payload: payloadHash, | ||
|
@@ -145,15 +145,64 @@ describe("Factory", () => { | |
|
||
const tx = await factory.execute( | ||
commandId, | ||
sourceContract, | ||
sourceChain, | ||
sourceAddress, | ||
payload, | ||
); | ||
|
||
await expect(tx) | ||
.to.emit(factory, "SmartWalletCreated") | ||
.withArgs(expectedWalletAddress, sourceAddress, "agoric", sourceAddress); | ||
await expect(tx).to.emit(factory, "CrossChainCallSent"); | ||
.to.emit(factory, "NewWalletCreated") | ||
.withArgs(expectedWalletAddress, nonce, sourceAddress, "agoric"); | ||
Comment on lines
+154
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks like the gist of it 👍 |
||
}); | ||
|
||
it("should revert if nonce is reused", async () => { | ||
const commandId = getCommandId(); | ||
|
||
const nonce = 1; | ||
const payload = abiCoder.encode(["uint256"], [nonce]); | ||
const payloadHash = keccak256(toBytes(payload)); | ||
|
||
await approveMessage({ | ||
commandId, | ||
from: sourceChain, | ||
sourceAddress, | ||
targetAddress: factory.target, | ||
payload: payloadHash, | ||
owner, | ||
AxelarGateway: axelarGatewayMock, | ||
abiCoder, | ||
}); | ||
|
||
await expect( | ||
factory.execute(commandId, sourceChain, sourceAddress, payload), | ||
).to.be.revertedWith("nonce already used by sender"); | ||
}); | ||
|
||
it("should revert if message comes from a chain besides agoric", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To what extent does this prevent forgery? Do we know that when it comes from Agoric, it's coming from an orchestration-controlled account? Or can anybody with a normal key-based account send such a message? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose if anyone wants to pay for such an account, we don't mind. |
||
const commandId = getCommandId(); | ||
const unsupportedSourceChain = "ethereum"; | ||
const payload = abiCoder.encode(["uint256"], [2]); | ||
const payloadHash = keccak256(toBytes(payload)); | ||
|
||
await approveMessage({ | ||
commandId, | ||
from: unsupportedSourceChain, | ||
sourceAddress, | ||
targetAddress: factory.target, | ||
payload: payloadHash, | ||
owner, | ||
AxelarGateway: axelarGatewayMock, | ||
abiCoder, | ||
}); | ||
|
||
await expect( | ||
factory.execute( | ||
commandId, | ||
unsupportedSourceChain, | ||
sourceAddress, | ||
payload, | ||
), | ||
).to.be.revertedWith("Only messages from Agoric chain are allowed"); | ||
}); | ||
|
||
it("should use the remote wallet to call other contracts", async () => { | ||
|
@@ -188,7 +237,7 @@ describe("Factory", () => { | |
const commandId1 = getCommandId(); | ||
await approveMessage({ | ||
commandId: commandId1, | ||
from: sourceContract, | ||
from: sourceChain, | ||
sourceAddress, | ||
targetAddress: wallet.target, | ||
payload: payloadHash, | ||
|
@@ -199,7 +248,7 @@ describe("Factory", () => { | |
|
||
const execTx = await wallet.execute( | ||
commandId1, | ||
sourceContract, | ||
sourceChain, | ||
sourceAddress, | ||
multicallPayload, | ||
); | ||
|
@@ -222,7 +271,7 @@ describe("Factory", () => { | |
const commandId2 = getCommandId(); | ||
await approveMessageWithToken({ | ||
commandId: commandId2, | ||
from: sourceContract, | ||
from: sourceChain, | ||
sourceAddress, | ||
targetAddress: wallet.target, | ||
payload: payloadHash2, | ||
|
@@ -235,7 +284,7 @@ describe("Factory", () => { | |
|
||
const execWithTokenTx = await wallet.executeWithToken( | ||
commandId2, | ||
sourceContract, | ||
sourceChain, | ||
sourceAddress, | ||
multicallPayload2, | ||
"USDC", | ||
|
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.
is this PR supposed to land with this change?