From 57023ec457545a754f3017a35357117a4fbf338a Mon Sep 17 00:00:00 2001 From: Sebastien La Duca Date: Fri, 10 Nov 2023 17:18:16 -0500 Subject: [PATCH] (bundler) check for transfers to sanctioned addresses (#576) * add erc20 parsing logic + scaffold sanction check * add sanctions list contract, wire up rest of checks * remove todo * changeset * fix typo * fix action parsing * switch rpc urls * rename --------- Co-authored-by: Luke Tchang --- .changeset/smart-paws-rule.md | 5 + actors/bundler/src/abis/SanctionsList.json | 188 ++++++++++++++++++ actors/bundler/src/actionParsing.ts | 38 ++++ actors/bundler/src/cli/commands/run/server.ts | 1 + actors/bundler/src/opValidation.ts | 37 ++++ actors/bundler/src/routes.ts | 13 ++ actors/bundler/src/sanctions.ts | 25 +++ actors/bundler/test/abis/ERC20.json | 117 +++++++++++ actors/bundler/test/actionDecoding.test.ts | 103 ++++++++++ actors/bundler/test/sanctions.test.ts | 33 +++ actors/bundler/test/utils.ts | 14 ++ packages/e2e-tests/src/hardhat.ts | 2 +- 12 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 .changeset/smart-paws-rule.md create mode 100644 actors/bundler/src/abis/SanctionsList.json create mode 100644 actors/bundler/src/actionParsing.ts create mode 100644 actors/bundler/src/sanctions.ts create mode 100644 actors/bundler/test/abis/ERC20.json create mode 100644 actors/bundler/test/actionDecoding.test.ts create mode 100644 actors/bundler/test/sanctions.test.ts diff --git a/.changeset/smart-paws-rule.md b/.changeset/smart-paws-rule.md new file mode 100644 index 000000000..dd5fbb3ac --- /dev/null +++ b/.changeset/smart-paws-rule.md @@ -0,0 +1,5 @@ +--- +"@nocturne-xyz/bundler": patch +--- + +check outgoing transfers against sanctions list diff --git a/actors/bundler/src/abis/SanctionsList.json b/actors/bundler/src/abis/SanctionsList.json new file mode 100644 index 000000000..4022c7ea6 --- /dev/null +++ b/actors/bundler/src/abis/SanctionsList.json @@ -0,0 +1,188 @@ +[ + { + "inputs": [], + "stateMutability": "nonpayable", + "type": "constructor" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "addr", + "type": "address" + } + ], + "name": "NonSanctionedAddress", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "previousOwner", + "type": "address" + }, + { + "indexed": true, + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "OwnershipTransferred", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "addr", + "type": "address" + } + ], + "name": "SanctionedAddress", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address[]", + "name": "addrs", + "type": "address[]" + } + ], + "name": "SanctionedAddressesAdded", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address[]", + "name": "addrs", + "type": "address[]" + } + ], + "name": "SanctionedAddressesRemoved", + "type": "event" + }, + { + "inputs": [ + { + "internalType": "address[]", + "name": "newSanctions", + "type": "address[]" + } + ], + "name": "addToSanctionsList", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "addr", + "type": "address" + } + ], + "name": "isSanctioned", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "addr", + "type": "address" + } + ], + "name": "isSanctionedVerbose", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "name", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "pure", + "type": "function" + }, + { + "inputs": [], + "name": "owner", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address[]", + "name": "removeSanctions", + "type": "address[]" + } + ], + "name": "removeFromSanctionsList", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "renounceOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "newOwner", + "type": "address" + } + ], + "name": "transferOwnership", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + } +] diff --git a/actors/bundler/src/actionParsing.ts b/actors/bundler/src/actionParsing.ts new file mode 100644 index 000000000..51c187534 --- /dev/null +++ b/actors/bundler/src/actionParsing.ts @@ -0,0 +1,38 @@ +import { Action } from "@nocturne-xyz/core"; +import * as ethers from "ethers"; + +export function getSelector(signature: string): string { + const sigBytes = ethers.utils.toUtf8Bytes(signature); + const hash = ethers.utils.keccak256(sigBytes); + return ethers.utils.hexDataSlice(hash, 0, 4); +} + +// same for both ETHTransferAdapter and ERC20 Transfer +const TRANSFER_SELECTOR = getSelector("transfer(address,uint256)"); + +export type TransferActionCalldata = { + to: string; + amount: string; +}; + +export function isTransferAction(action: Action): boolean { + const selector = ethers.utils.hexDataSlice(action.encodedFunction, 0, 4); + return selector === TRANSFER_SELECTOR; +} + +export function parseTransferAction(action: Action): TransferActionCalldata { + if (!isTransferAction(action)) { + throw new Error("Not an ERC20 transfer action"); + } + + const calldata = ethers.utils.hexDataSlice(action.encodedFunction, 4); + const [to, amount] = ethers.utils.defaultAbiCoder.decode( + ["address", "uint256"], + calldata + ) as [string, ethers.BigNumber]; + + return { + to, + amount: amount.toString(), + }; +} diff --git a/actors/bundler/src/cli/commands/run/server.ts b/actors/bundler/src/cli/commands/run/server.ts index 17c462ec9..21b8a3a97 100644 --- a/actors/bundler/src/cli/commands/run/server.ts +++ b/actors/bundler/src/cli/commands/run/server.ts @@ -40,6 +40,7 @@ const runServer = new Command("server") logLevel, logDir ); + const server = new BundlerServer( bundlerAddress, config.tellerAddress, diff --git a/actors/bundler/src/opValidation.ts b/actors/bundler/src/opValidation.ts index 603a6ccd1..c1bf59e2b 100644 --- a/actors/bundler/src/opValidation.ts +++ b/actors/bundler/src/opValidation.ts @@ -8,6 +8,8 @@ import { Handler, Teller } from "@nocturne-xyz/contracts"; import { NullifierDB } from "./db"; import { Logger } from "winston"; import { ErrString } from "@nocturne-xyz/offchain-utils"; +import { isTransferAction, parseTransferAction } from "./actionParsing"; +import { isSanctionedAddress } from "./sanctions"; export async function checkNullifierConflictError( db: NullifierDB, @@ -102,3 +104,38 @@ export async function checkNotEnoughGasError( return `operation ${id} gas price too low: ${operation.gasPrice} < current chain's gas price ${gasPrice}`; } } + +export async function checkIsNotTransferToSanctionedAddress( + provider: ethers.providers.Provider, + logger: Logger, + operation: SubmittableOperationWithNetworkInfo +): Promise { + logger.debug( + "checking that operation doesn't contain any transfers to a sanctioned address" + ); + + const transferActions = operation.actions.filter(isTransferAction); + const opDigest = OperationTrait.computeDigest(operation).toString(); + const results = await Promise.all( + transferActions.map(async (action, i) => { + const { to, amount } = parseTransferAction(action); + if (await isSanctionedAddress(provider, to)) { + logger.alert("detected transfer to sanctioned address", { + opDigest, + actionIndex: i, + recipient: to, + amount, + contract: action.contractAddress, + }); + return true; + } + + return false; + }) + ); + + const sanctionedTransfers = results.filter((result) => result === true); + if (sanctionedTransfers.length > 0) { + return `operation ${opDigest} contains ${sanctionedTransfers.length} transfer(s) to sanctioned addresses`; + } +} diff --git a/actors/bundler/src/routes.ts b/actors/bundler/src/routes.ts index b09edb333..eb54df225 100644 --- a/actors/bundler/src/routes.ts +++ b/actors/bundler/src/routes.ts @@ -16,6 +16,7 @@ import { } from "@nocturne-xyz/core"; import { Handler, Teller } from "@nocturne-xyz/contracts"; import { + checkIsNotTransferToSanctionedAddress, checkNotEnoughGasError, checkNullifierConflictError, checkRevertError, @@ -124,6 +125,18 @@ export function makeRelayHandler({ return; } + const sanctionedTransferErr = await checkIsNotTransferToSanctionedAddress( + provider, + logger, + operation + ); + if (sanctionedTransferErr) { + logValidationFailure(sanctionedTransferErr); + // TODO: add histogram for sanctioned transfers? + res.status(400).json(sanctionedTransferErr); + return; + } + // Enqueue operation and add all inflight nullifiers let jobId; try { diff --git a/actors/bundler/src/sanctions.ts b/actors/bundler/src/sanctions.ts new file mode 100644 index 000000000..30f19bdd1 --- /dev/null +++ b/actors/bundler/src/sanctions.ts @@ -0,0 +1,25 @@ +import { Address } from "@nocturne-xyz/core"; +import * as ethers from "ethers"; +import SanctionsListAbi from "./abis/SanctionsList.json"; + +const CHAIN_ID_TO_SANCTIONS_LIST_CONTRACT: Record = { + 1: "0x40c57923924b5c5c5455c48d93317139addac8fb", +}; + +export async function isSanctionedAddress( + provider: ethers.providers.Provider, + address: Address +): Promise { + const chainId = (await provider.getNetwork()).chainId; + // skip check if there's no sanctions contract on this chain + if (!CHAIN_ID_TO_SANCTIONS_LIST_CONTRACT[chainId]) { + return false; + } + + const contract = new ethers.Contract( + CHAIN_ID_TO_SANCTIONS_LIST_CONTRACT[chainId], + SanctionsListAbi, + provider + ); + return (await contract.isSanctioned(address)) as unknown as boolean; +} diff --git a/actors/bundler/test/abis/ERC20.json b/actors/bundler/test/abis/ERC20.json new file mode 100644 index 000000000..dd7bf99d6 --- /dev/null +++ b/actors/bundler/test/abis/ERC20.json @@ -0,0 +1,117 @@ +[ + { + "constant": true, + "inputs": [], + "name": "name", + "outputs": [{ "name": "", "type": "string" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { + "constant": false, + "inputs": [ + { "name": "_spender", "type": "address" }, + { "name": "_value", "type": "uint256" } + ], + "name": "approve", + "outputs": [{ "name": "", "type": "bool" }], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" + }, + { + "constant": true, + "inputs": [], + "name": "totalSupply", + "outputs": [{ "name": "", "type": "uint256" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { + "constant": false, + "inputs": [ + { "name": "_from", "type": "address" }, + { "name": "_to", "type": "address" }, + { "name": "_value", "type": "uint256" } + ], + "name": "transferFrom", + "outputs": [{ "name": "", "type": "bool" }], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" + }, + { + "constant": true, + "inputs": [], + "name": "decimals", + "outputs": [{ "name": "", "type": "uint8" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { + "constant": true, + "inputs": [{ "name": "_owner", "type": "address" }], + "name": "balanceOf", + "outputs": [{ "name": "balance", "type": "uint256" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { + "constant": true, + "inputs": [], + "name": "symbol", + "outputs": [{ "name": "", "type": "string" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { + "constant": false, + "inputs": [ + { "name": "_to", "type": "address" }, + { "name": "_value", "type": "uint256" } + ], + "name": "transfer", + "outputs": [{ "name": "", "type": "bool" }], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" + }, + { + "constant": true, + "inputs": [ + { "name": "_owner", "type": "address" }, + { "name": "_spender", "type": "address" } + ], + "name": "allowance", + "outputs": [{ "name": "", "type": "uint256" }], + "payable": false, + "stateMutability": "view", + "type": "function" + }, + { "payable": true, "stateMutability": "payable", "type": "fallback" }, + { + "anonymous": false, + "inputs": [ + { "indexed": true, "name": "owner", "type": "address" }, + { "indexed": true, "name": "spender", "type": "address" }, + { "indexed": false, "name": "value", "type": "uint256" } + ], + "name": "Approval", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { "indexed": true, "name": "from", "type": "address" }, + { "indexed": true, "name": "to", "type": "address" }, + { "indexed": false, "name": "value", "type": "uint256" } + ], + "name": "Transfer", + "type": "event" + } +] diff --git a/actors/bundler/test/actionDecoding.test.ts b/actors/bundler/test/actionDecoding.test.ts new file mode 100644 index 000000000..78bf98fc8 --- /dev/null +++ b/actors/bundler/test/actionDecoding.test.ts @@ -0,0 +1,103 @@ +import "mocha"; +import { expect } from "chai"; +import * as ethers from "ethers"; +import { Action, Address } from "@nocturne-xyz/core"; +import { + EthTransferAdapter__factory, + WstethAdapter__factory, +} from "@nocturne-xyz/contracts"; +import { DUMMY_ADDRESSES, DUMMY_CONTRACT_ADDRESS } from "./utils"; +import { isTransferAction, parseTransferAction } from "../src/actionParsing"; +import ERC20_ABI from "./abis/ERC20.json"; + +describe("Action Decoding", () => { + it("detects transfer actions", () => { + const ethTransferAction = dummyEthTransferAction(DUMMY_ADDRESSES[0], 100n); + expect(isTransferAction(ethTransferAction)).to.be.true; + + const erc20TransferAction = dummyErc20TransferAction( + DUMMY_ADDRESSES[0], + 100n + ); + expect(isTransferAction(erc20TransferAction)).to.be.true; + + const erc20ApproveAction = dummyErc20ApproveAction( + DUMMY_ADDRESSES[0], + 100n + ); + expect(isTransferAction(erc20ApproveAction)).to.be.false; + + const wstethDepositAction = dummyWstethDepositAction(100n); + expect(isTransferAction(wstethDepositAction)).to.be.false; + }); + + it("correctly parses ETH transfer actions", () => { + const ethTransferAction = dummyEthTransferAction(DUMMY_ADDRESSES[0], 100n); + const { to, amount } = parseTransferAction(ethTransferAction); + expect(to).to.equal(DUMMY_ADDRESSES[0]); + expect(amount).to.equal("100"); + }); + + it("correctly parses ERC20 transfer actions", () => { + const erc20TransferAction = dummyErc20TransferAction( + DUMMY_ADDRESSES[0], + 100n + ); + const { to, amount } = parseTransferAction(erc20TransferAction); + expect(to).to.equal(DUMMY_ADDRESSES[0]); + expect(amount).to.equal("100"); + }); +}); + +function dummyEthTransferAction(recipient: Address, amount: bigint): Action { + const encodedFunction = + EthTransferAdapter__factory.createInterface().encodeFunctionData( + "transfer", + [recipient, amount] + ); + + return { + contractAddress: DUMMY_CONTRACT_ADDRESS, + encodedFunction, + }; +} + +function dummyErc20TransferAction(recipient: Address, amount: bigint): Action { + const contract = new ethers.Contract(DUMMY_CONTRACT_ADDRESS, ERC20_ABI); + + const encodedFunction = contract.interface.encodeFunctionData("transfer", [ + recipient, + amount, + ]); + + return { + contractAddress: DUMMY_CONTRACT_ADDRESS, + encodedFunction, + }; +} + +function dummyErc20ApproveAction(spender: Address, amount: bigint): Action { + const contract = new ethers.Contract(DUMMY_CONTRACT_ADDRESS, ERC20_ABI); + + const encodedFunction = contract.interface.encodeFunctionData("approve", [ + spender, + amount, + ]); + + return { + contractAddress: DUMMY_CONTRACT_ADDRESS, + encodedFunction, + }; +} + +function dummyWstethDepositAction(amount: bigint): Action { + const encodedFunction = + WstethAdapter__factory.createInterface().encodeFunctionData("deposit", [ + amount, + ]); + + return { + contractAddress: DUMMY_CONTRACT_ADDRESS, + encodedFunction, + }; +} diff --git a/actors/bundler/test/sanctions.test.ts b/actors/bundler/test/sanctions.test.ts new file mode 100644 index 000000000..414857077 --- /dev/null +++ b/actors/bundler/test/sanctions.test.ts @@ -0,0 +1,33 @@ +import "mocha"; +import { expect } from "chai"; +import { isSanctionedAddress } from "../src/sanctions"; +import { TEST_PROVIDER } from "./utils"; + +// if we have trouble, pull in from env and setup env vars + +describe("SanctionsList", () => { + // sleep to avoid rate limits + afterEach(async () => { + await new Promise((resolve) => setTimeout(resolve, 5000)); + }); + + it("returns true for sanctioned address", async () => { + // test case from chainalysis article here: https://go.chainalysis.com/chainalysis-oracle-docs.html + const res = await isSanctionedAddress( + TEST_PROVIDER, + "0x7F367cC41522cE07553e823bf3be79A889DEbe1B" + ); + + expect(res).to.be.true; + }); + + it("returns false for non-sanctioned address", async () => { + // test case from chainalysis article here: https://go.chainalysis.com/chainalysis-oracle-docs.html + const res = await isSanctionedAddress( + TEST_PROVIDER, + "0x7f268357A8c2552623316e2562D90e642bB538E5" + ); + + expect(res).to.be.false; + }); +}); diff --git a/actors/bundler/test/utils.ts b/actors/bundler/test/utils.ts index 43ce25f30..76160e8b6 100644 --- a/actors/bundler/test/utils.ts +++ b/actors/bundler/test/utils.ts @@ -1,3 +1,5 @@ +import { ethers } from "ethers"; + const JOINSPLIT = { proof: ["0n", "0n", "0n", "0n", "0n", "0n", "0n", "0n"], senderCommitment: "0n", @@ -61,3 +63,15 @@ export const VALID_RELAY_REQUEST = { atomicActions: true, }, }; + +const RPC_URL = + "https://eth-mainnet.g.alchemy.com/v2/oaZgSEuM_n2GWITRCIJ3-pO-aKGtYC0l"; +export const TEST_PROVIDER = new ethers.providers.JsonRpcProvider(RPC_URL); +export const DUMMY_CONTRACT_ADDRESS = ethers.utils.getAddress( + "0x71C7656EC7ab88b098defB751B7401B5f6d8976F" +); +export const DUMMY_ADDRESSES = [ + ethers.utils.getAddress("0xddbd1e80090943632ed47b1632cb36e7ca28abc2"), + ethers.utils.getAddress("0x6798639591530fbbafd12c2826422b58bd2c5219"), + ethers.utils.getAddress("0x67f8f9a5d4290325506b119980660624dc7d3ba9"), +]; diff --git a/packages/e2e-tests/src/hardhat.ts b/packages/e2e-tests/src/hardhat.ts index fdf5f35d5..fb6116c5f 100644 --- a/packages/e2e-tests/src/hardhat.ts +++ b/packages/e2e-tests/src/hardhat.ts @@ -16,7 +16,7 @@ export type ForkNetwork = typeof FORK_NETWORKS; const FORK_NETWORK_MAPPING: { [K in ForkNetwork]: string } = { mainnet: - "https://eth-mainnet.g.alchemy.com/v2/X21iuJe_hcEAH4cooxG7xGuTQ-ebJJmB", + "https://eth-mainnet.g.alchemy.com/v2/oaZgSEuM_n2GWITRCIJ3-pO-aKGtYC0l", }; // returns snapshotId of empty chain state