test: multi-chain verifiable merkle tree #688
Conversation
Signed-off-by: Pablo Maldonado <[email protected]>
| @@ -103,7 +179,7 @@ export const relayerRefundHashFn = (input: RelayerRefundLeaf | RelayerRefundLeaf | |||
| const abiCoder = new ethers.utils.AbiCoder(); | |||
| const encodedData = abiCoder.encode( | |||
| [ | |||
| "tuple(uint256 leafId, uint256 chainId, uint256 amountToReturn, address l2TokenAddress, address[] refundAddresses, uint256[] refundAmounts)", | |||
| "tuple( uint256 amountToReturn, uint256 chainId, uint256[] refundAmounts, uint256 leafId, address l2TokenAddress, address[] refundAddresses)", | |||
There was a problem hiding this comment.
The order wasn't matching the one in EVM tests.
There was a problem hiding this comment.
ye, this is the thing I was trying to say in the call on Tuesday: we should make them match within the rust/solidity code! there is no reason from them not to match, its just extra complexity. I would suggest we update the rust to match the solidity ordering, even with differently named fields
There was a problem hiding this comment.
Yes, makes sense @chrismaree. I will work in the svm changes in a subsequent PR 👍
Signed-off-by: Pablo Maldonado <[email protected]>
| @@ -1,4 +1,5 @@ | |||
| import { SignerWithAddress, seedContract, toBN, expect, Contract, ethers, BigNumber } from "../../../utils/utils"; | |||
| import { buildRelayerRefundMerkleTree } from "../../svm/utils"; | |||
There was a problem hiding this comment.
this is great, I really like this pattern that you have here being able to mix the EVM/SVM tests via the tree building logic.
| amountToReturn: BigNumber.from(0), | ||
| l2TokenAddress: evmTokenAddress ?? randomAddress(), | ||
| refundAddresses: evmRelayers || [randomAddress(), randomAddress()], | ||
| refundAmounts: evmRefundAmounts || [BigNumber.from(randomBigInt()), BigNumber.from(randomBigInt())], |
There was a problem hiding this comment.
it's generally a code smell to have random numbers within unit tests but I think for this it's ok.
test/svm/SvmSpoke.Bundle.ts
Outdated
| @@ -9,26 +15,19 @@ import { | |||
| TransactionMessage, | |||
| VersionedTransaction, | |||
| } from "@solana/web3.js"; | |||
| import { MerkleTree } from "@uma/common/dist/MerkleTree"; | |||
There was a problem hiding this comment.
nit: better import from utils/MerkleTree.ts. Though under the hood its the same uma common package import, it might be better to import it consistently from one path
Signed-off-by: Pablo Maldonado <[email protected]>
md0x
deleted the
pablo/acx-3015-instructionsbundlers-add-multichain-test-to-check-if-its
branch
Changes proposed in this PR: