From e877973d2ac8c6eb38476431255d65fd8e366d6f Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 09:34:13 +0000 Subject: [PATCH 1/7] fix: remove bytecode check --- src/domain/events/index.ts | 9 +-------- src/domain/polling/index.ts | 31 +++++++++++++++++++++---------- src/utils/contracts.ts | 30 ------------------------------ src/utils/utils.spec.ts | 36 ------------------------------------ 4 files changed, 22 insertions(+), 84 deletions(-) diff --git a/src/domain/events/index.ts b/src/domain/events/index.ts index 287b741..ce00277 100644 --- a/src/domain/events/index.ts +++ b/src/domain/events/index.ts @@ -2,7 +2,6 @@ import { toConditionalOrderParams, getLogger, handleExecutionError, - isComposableCowCompatible, metrics, } from "../../utils"; import { BytesLike, ethers } from "ethers"; @@ -50,19 +49,13 @@ async function _addContract( ) { const log = getLogger("addContract:_addContract"); const composableCow = ComposableCoW__factory.createInterface(); - const { provider, registry } = context; + const { registry } = context; const { transactionHash: tx, blockNumber } = event; // Process the logs let hasErrors = false; let numContractsAdded = 0; - // Do not process logs that are not from a `ComposableCoW`-compatible contract - // This is a *normal* case, if the contract is not `ComposableCoW`-compatible - // then we do not need to do anything, and therefore don't flag as an error. - if (!isComposableCowCompatible(await provider.getCode(event.address))) { - return; - } const { error, added } = await _registerNewOrder( event, composableCow, diff --git a/src/domain/polling/index.ts b/src/domain/polling/index.ts index a8f0752..ed71b21 100644 --- a/src/domain/polling/index.ts +++ b/src/domain/polling/index.ts @@ -690,16 +690,27 @@ async function _pollLegacy( const [{ success, returnData }] = lowLevelCall; if (success) { - // Decode the returnData to get the order and signature tuple - const { order, signature } = contract.interface.decodeFunctionResult( - "getTradeableOrderWithSignature", - returnData - ); - return { - result: PollResultCode.SUCCESS, - order, - signature, - }; + try { + // Decode the returnData to get the order and signature tuple + const { order, signature } = contract.interface.decodeFunctionResult( + "getTradeableOrderWithSignature", + returnData + ); + return { + result: PollResultCode.SUCCESS, + order, + signature, + }; + } catch (error: any) { + log.error(`ethers/decodeFunctionResult Unexpected error`, error); + metrics.pollingOnChainEthersErrorsTotal.labels(...metricLabels).inc(); + return { + result: PollResultCode.DONT_TRY_AGAIN, + reason: + "UnexpectedErrorName: Unexpected error" + + (error.message ? `: ${error.message}` : ""), + }; + } } // If the low-level call failed, per the `ComposableCoW` interface, the contract is attempting to diff --git a/src/utils/contracts.ts b/src/utils/contracts.ts index 47d718b..0bc6a18 100644 --- a/src/utils/contracts.ts +++ b/src/utils/contracts.ts @@ -10,12 +10,6 @@ import { import { getLogger } from "./logging"; import { metrics } from "."; -// Selectors that are required to be part of the contract's bytecode in order to be considered compatible -const REQUIRED_SELECTORS = [ - "cabinet(address,bytes32)", - "getTradeableOrderWithSignature(address,(address,bytes32,bytes),bytes,bytes32[])", -]; - // Define an enum for the custom error revert hints that can be returned by the ComposableCoW's interfaces. export enum CustomErrorSelectors { /** @@ -117,30 +111,6 @@ export function abiToSelector(abi: string) { return ethers.utils.id(abi).slice(0, 10); } -/** - * Attempts to verify that the contract at the given address implements the interface of the `ComposableCoW` - * contract. This is done by checking that the contract contains the selectors of the functions that are - * required to be implemented by the interface. - * - * @remarks This is not a foolproof way of verifying that the contract implements the interface, but it is - * a good enough heuristic to filter out most of the contracts that do not implement the interface. - * - * @dev The selectors are: - * - `cabinet(address,bytes32)`: `1c7662c8` - * - `getTradeableOrderWithSignature(address,(address,bytes32,bytes),bytes,bytes32[])`: `26e0a196` - * - * @param code the contract's deployed bytecode as a hex string - * @returns A boolean indicating if the contract likely implements the interface - */ -export function isComposableCowCompatible(code: string): boolean { - const composableCow = ComposableCoW__factory.createInterface(); - - return REQUIRED_SELECTORS.every((signature) => { - const sighash = composableCow.getSighash(signature); - return code.includes(sighash.slice(2)); - }); -} - export function composableCowContract( provider: ethers.providers.Provider, chainId: SupportedChainId diff --git a/src/utils/utils.spec.ts b/src/utils/utils.spec.ts index ae666a4..8c2af3d 100644 --- a/src/utils/utils.spec.ts +++ b/src/utils/utils.spec.ts @@ -1,49 +1,13 @@ -import * as composableCow from "../../abi/ComposableCoW.json"; -import * as extensibleFallbackHandler from "../../abi/ExtensibleFallbackHandler.json"; import { CUSTOM_ERROR_ABI_MAP, CustomErrorSelectors, abiToSelector, handleOnChainCustomError, initLogging, - isComposableCowCompatible, parseCustomError, } from "."; import { COMPOSABLE_COW_CONTRACT_ADDRESS } from "@cowprotocol/cow-sdk"; -// consts for readability -const composableCowBytecode = composableCow.deployedBytecode.object; -const failBytecode = extensibleFallbackHandler.deployedBytecode.object; - -describe("test supports composable cow interface from bytecode", () => { - it("should pass", () => { - expect(isComposableCowCompatible(composableCowBytecode)).toBe(true); - }); - - it("should fail", () => { - expect(isComposableCowCompatible(failBytecode)).toBe(false); - }); -}); - -describe("test against concrete examples", () => { - const signatures = ["0x1c7662c8", "0x26e0a196"]; - - it("should pass with both selectors", () => { - expect(isComposableCowCompatible("0x1c7662c826e0a196")).toBe(true); - }); - - // using `forEach` here, be careful not to do async tests. - signatures.forEach((s) => { - it(`should fail with only selector ${s}`, () => { - expect(isComposableCowCompatible(s)).toBe(false); - }); - }); - - it("should fail with no selectors", () => { - expect(isComposableCowCompatible("0xdeadbeefdeadbeef")).toBe(false); - }); -}); - describe("parse custom errors (reversions)", () => { it("should pass the SingleOrderNotAuthed selector correctly", () => { expect(parseCustomError(SINGLE_ORDER_NOT_AUTHED_ERROR)).toMatchObject({ From c98304d8b9d07c2bcc8550565a319569b727d59e Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 09:35:58 +0000 Subject: [PATCH 2/7] chore: version minor bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a57b102..cbc9106 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@cowprotocol/watch-tower", "license": "GPL-3.0-or-later", - "version": "1.2.0", + "version": "1.3.0", "description": "A standalone watch tower, keeping an eye on Composable Cows 👀🐮", "author": { "name": "Cow Protocol" From 9da1c07f9dfdfc7d105dbdd034f36e113ec51afb Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 12:05:14 +0000 Subject: [PATCH 3/7] fix: not-address specific log checking --- src/services/chain.ts | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/src/services/chain.ts b/src/services/chain.ts index 9543417..d71cdca 100644 --- a/src/services/chain.ts +++ b/src/services/chain.ts @@ -16,7 +16,7 @@ import { } from "@cowprotocol/cow-sdk"; import { addContract } from "../domain/events"; import { checkForAndPlaceOrder } from "../domain/polling"; -import { EventFilter, providers } from "ethers"; +import { ethers, providers } from "ethers"; import { composableCowContract, getLogger, @@ -24,7 +24,6 @@ import { metrics, } from "../utils"; import { DBService } from "."; -import { hexZeroPad } from "ethers/lib/utils"; import { policy } from "../domain/polling/filtering"; const WATCHDOG_FREQUENCY_SECS = 5; // 5 seconds @@ -506,22 +505,37 @@ async function processBlock( } } -function pollContractForEvents( +async function pollContractForEvents( fromBlock: number, toBlock: number | "latest", context: ChainContext ): Promise { - const { provider, chainId, addresses } = context; + const { provider, chainId } = context; const composableCow = composableCowContract(provider, chainId); - const filter = composableCow.filters.ConditionalOrderCreated() as EventFilter; + const eventName = "ConditionalOrderCreated(address,(address,bytes32,bytes))"; + const topic = ethers.utils.id(eventName); - if (addresses) { - filter.topics?.push( - addresses.map((address) => hexZeroPad(address.toLowerCase(), 32)) - ); - } + const logs = await provider.getLogs({ + fromBlock, + toBlock, + topics: [topic], + }); + + const returnLogs = logs + .map((e) => { + try { + return composableCow.interface.decodeEventLog( + topic, + e.data, + e.topics + ) as unknown as ConditionalOrderCreatedEvent; + } catch { + return null; + } + }) + .filter((e): e is ConditionalOrderCreatedEvent => e !== null); - return composableCow.queryFilter(filter, fromBlock, toBlock); + return returnLogs; } function _formatResult(result: boolean) { From 2555b3855b48b7a1694c2064d60127a0d46dfdb8 Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 12:47:48 +0000 Subject: [PATCH 4/7] fix: event decoding to keep event metadata --- src/services/chain.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/services/chain.ts b/src/services/chain.ts index d71cdca..a5a3e11 100644 --- a/src/services/chain.ts +++ b/src/services/chain.ts @@ -521,21 +521,24 @@ async function pollContractForEvents( topics: [topic], }); - const returnLogs = logs - .map((e) => { + return logs + .map((event) => { try { - return composableCow.interface.decodeEventLog( + const decoded = composableCow.interface.decodeEventLog( topic, - e.data, - e.topics + event.data, + event.topics ) as unknown as ConditionalOrderCreatedEvent; + + return { + ...decoded, + ...event, + }; } catch { return null; } }) .filter((e): e is ConditionalOrderCreatedEvent => e !== null); - - return returnLogs; } function _formatResult(result: boolean) { From dd7c0355b2b84fb9d450a8f6d01a5c2897bff055 Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 13:30:29 +0000 Subject: [PATCH 5/7] fix: target address from database field --- src/domain/polling/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/polling/index.ts b/src/domain/polling/index.ts index ed71b21..7813657 100644 --- a/src/domain/polling/index.ts +++ b/src/domain/polling/index.ts @@ -665,11 +665,11 @@ async function _pollLegacy( ): Promise { const { contract, multicall, chainId } = context; const log = getLogger("checkForAndPlaceOrder:_pollLegacy", orderRef); + const { composableCow: target } = conditionalOrder; const { handler } = conditionalOrder.params; // as we going to use multicall, with `aggregate3Value`, there is no need to do any simulation as the // calls are guaranteed to pass, and will return the results, or the reversion within the ABI-encoded data. // By not using `populateTransaction`, we avoid an `eth_estimateGas` RPC call. - const target = contract.address; const callData = contract.interface.encodeFunctionData( "getTradeableOrderWithSignature", [owner, conditionalOrder.params, offchainInput, proof] From 93f6be99962a281d4c8ee437d93fe204326d743b Mon Sep 17 00:00:00 2001 From: mfw78 <53399572+mfw78@users.noreply.github.com> Date: Fri, 10 May 2024 13:36:37 +0000 Subject: [PATCH 6/7] Update src/domain/polling/index.ts Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com> --- src/domain/polling/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/polling/index.ts b/src/domain/polling/index.ts index 7813657..6eb739e 100644 --- a/src/domain/polling/index.ts +++ b/src/domain/polling/index.ts @@ -707,7 +707,7 @@ async function _pollLegacy( return { result: PollResultCode.DONT_TRY_AGAIN, reason: - "UnexpectedErrorName: Unexpected error" + + "UnexpectedErrorName: Data decoding failure" + (error.message ? `: ${error.message}` : ""), }; } From 78ff4b86ae41d63c3cc438c60349423f3bea42b9 Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 10 May 2024 13:58:26 +0000 Subject: [PATCH 7/7] fix: add in missing addresses filtering --- src/services/chain.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/services/chain.ts b/src/services/chain.ts index a5a3e11..872a256 100644 --- a/src/services/chain.ts +++ b/src/services/chain.ts @@ -510,7 +510,7 @@ async function pollContractForEvents( toBlock: number | "latest", context: ChainContext ): Promise { - const { provider, chainId } = context; + const { provider, chainId, addresses } = context; const composableCow = composableCowContract(provider, chainId); const eventName = "ConditionalOrderCreated(address,(address,bytes32,bytes))"; const topic = ethers.utils.id(eventName); @@ -538,7 +538,10 @@ async function pollContractForEvents( return null; } }) - .filter((e): e is ConditionalOrderCreatedEvent => e !== null); + .filter((e): e is ConditionalOrderCreatedEvent => e !== null) + .filter((e): e is ConditionalOrderCreatedEvent => { + return addresses ? addresses.includes(e.args.owner) : true; + }); } function _formatResult(result: boolean) {