From 30a6ad33635857e839d62e1bbc7d92129299b391 Mon Sep 17 00:00:00 2001 From: Benjamin Smith Date: Mon, 20 Feb 2023 14:22:46 +0100 Subject: [PATCH] Disable Transfer Consolidation (#194) We will spend more gas to do individual transfers for all Rewards, Gas Reimbursements and Positive slippage. This is supposed to make it easier to validate the solver payouts and in some sense it kinda does. The records are sorted in the same way they will appear in the dashboard. --- src/fetch/dune.py | 2 +- src/fetch/transfer_file.py | 42 ++-- src/models/overdraft.py | 4 +- src/models/split_transfers.py | 10 +- src/models/transfer.py | 79 +++++-- src/utils/script_args.py | 9 + tests/e2e/test_post_transaction.py | 4 +- tests/e2e/test_transfer_file.py | 10 +- tests/unit/test_models.py | 347 +++++++++++++++++++++++++---- tests/unit/test_multisend.py | 8 +- tests/unit/test_split_transfer.py | 67 +++--- tests/unit/util_methods.py | 10 + 12 files changed, 467 insertions(+), 125 deletions(-) create mode 100644 tests/unit/util_methods.py diff --git a/src/fetch/dune.py b/src/fetch/dune.py index abee8bad..a53221c4 100644 --- a/src/fetch/dune.py +++ b/src/fetch/dune.py @@ -59,7 +59,7 @@ def _parameterized_query( def _get_query_results(self, query: Query) -> list[dict[str, str]]: """Internally every dune query execution is routed through here.""" log.info(f"Fetching {query.name} from query: {query}") - exec_result = self.dune.refresh(query, ping_frequency=5) + exec_result = self.dune.refresh(query, ping_frequency=15) log.info(f"Fetch completed for execution {exec_result.execution_id}") # TODO - use a real logger: # https://github.com/cowprotocol/dune-client/issues/34 diff --git a/src/fetch/transfer_file.py b/src/fetch/transfer_file.py index 0cb84896..fb25bac0 100644 --- a/src/fetch/transfer_file.py +++ b/src/fetch/transfer_file.py @@ -21,26 +21,25 @@ SAFE_URL, FILE_OUT_DIR, ) -from src.fetch.dune import DuneFetcher +from src.models.accounting_period import AccountingPeriod from src.models.transfer import Transfer, CSVTransfer from src.multisend import post_multisend, prepend_unwrap_if_necessary from src.slack import post_to_slack -from src.utils.print_store import Category +from src.utils.print_store import Category, PrintStore from src.utils.script_args import generic_script_init -def manual_propose(dune: DuneFetcher) -> None: +def manual_propose(transfers: list[Transfer], period: AccountingPeriod) -> None: """ Entry point to manual creation of rewards payout transaction. This function generates the CSV transfer file to be pasted into the COW Safe app """ print( f"Please double check the batches with unusual slippage: " - f"{dune.period.unusual_slippage_url()}" + f"{period.unusual_slippage_url()}" ) - transfers = Transfer.consolidate(dune.get_transfers()) csv_transfers = [asdict(CSVTransfer.from_transfer(t)) for t in transfers] - FileIO(FILE_OUT_DIR).write_csv(csv_transfers, f"transfers-{dune.period}.csv") + FileIO(FILE_OUT_DIR).write_csv(csv_transfers, f"transfers-{period}.csv") print(Transfer.summarize(transfers)) print( @@ -50,7 +49,12 @@ def manual_propose(dune: DuneFetcher) -> None: ) -def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> None: +def auto_propose( + transfers: list[Transfer], + log_saver: PrintStore, + slack_client: WebClient, + dry_run: bool, +) -> None: """ Entry point auto creation of rewards payout transaction. This function encodes the multisend of reward transfers and posts @@ -61,13 +65,12 @@ def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> N signing_key = os.environ["PROPOSER_PK"] client = EthereumClient(URI(NODE_URL)) - transfers = Transfer.consolidate(dune.get_transfers()) - dune.log_saver.print(Transfer.summarize(transfers), category=Category.TOTALS) + log_saver.print(Transfer.summarize(transfers), category=Category.TOTALS) transactions = prepend_unwrap_if_necessary( client, SAFE_ADDRESS, transactions=[t.as_multisend_tx() for t in transfers] ) if len(transactions) > len(transfers): - dune.log_saver.print("Prepended WETH unwrap", Category.GENERAL) + log_saver.print("Prepended WETH unwrap", Category.GENERAL) if not dry_run: nonce = post_multisend( @@ -85,20 +88,27 @@ def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> N f"To sign and execute, visit:\n{SAFE_URL}\n" f"More details in thread" ), - sub_messages=dune.log_saver.get_values(), + sub_messages=log_saver.get_values(), ) if __name__ == "__main__": args = generic_script_init(description="Fetch Complete Reimbursement") - args.dune.log_saver.print( - f"The data aggregated can be visualized at\n" - f"{args.dune.period.dashboard_url()}", + dune = args.dune + dune.log_saver.print( + f"The data aggregated can be visualized at\n" f"{dune.period.dashboard_url()}", category=Category.GENERAL, ) + + payout_transfers = dune.get_transfers() + Transfer.sort_list(payout_transfers) + if args.consolidate_transfers: + payout_transfers = Transfer.consolidate(payout_transfers) + if args.post_tx: auto_propose( - dune=args.dune, + transfers=payout_transfers, + log_saver=dune.log_saver, slack_client=WebClient( token=os.environ["SLACK_TOKEN"], # https://stackoverflow.com/questions/59808346/python-3-slack-client-ssl-sslcertverificationerror @@ -107,4 +117,4 @@ def auto_propose(dune: DuneFetcher, slack_client: WebClient, dry_run: bool) -> N dry_run=args.dry_run, ) else: - manual_propose(args.dune) + manual_propose(transfers=payout_transfers, period=dune.period) diff --git a/src/models/overdraft.py b/src/models/overdraft.py index 25eb5499..f294ec2a 100644 --- a/src/models/overdraft.py +++ b/src/models/overdraft.py @@ -28,7 +28,7 @@ def from_objects( cls, transfer: Transfer, slippage: SolverSlippage, period: AccountingPeriod ) -> Overdraft: """Constructs an overdraft instance based on Transfer & Slippage""" - assert transfer.receiver == slippage.solver_address + assert transfer.solver == slippage.solver_address assert transfer.token_type == TokenType.NATIVE overdraft = transfer.amount_wei + slippage.amount_wei assert overdraft < 0, "This is why we are here." @@ -46,6 +46,6 @@ def eth(self) -> float: def __str__(self) -> str: return ( - f"Overdraft(solver={self.account}({self.name})," + f"Overdraft(solver={self.account} ({self.name})," f"period={self.period},owed={self.eth} ETH)" ) diff --git a/src/models/split_transfers.py b/src/models/split_transfers.py index 19447468..395aa00a 100644 --- a/src/models/split_transfers.py +++ b/src/models/split_transfers.py @@ -61,7 +61,7 @@ def _process_native_transfers( penalty_total = 0 while self.unprocessed_native: transfer = self.unprocessed_native.pop(0) - solver = transfer.receiver + solver = transfer.solver slippage: Optional[SolverSlippage] = indexed_slippage.get(solver) if slippage is not None: assert ( @@ -73,7 +73,7 @@ def _process_native_transfers( except ValueError as err: name, address = slippage.solver_name, slippage.solver_address self.log_saver.print( - f"Slippage for {address}({name}) exceeds reimbursement: {err}\n" + f"Slippage for {address} ({name}) exceeds reimbursement: {err}\n" f"Excluding payout and appending excess to overdraft", category=Category.OVERDRAFT, ) @@ -99,7 +99,7 @@ def _process_rewards( price_day = self.period.end - timedelta(days=1) while self.unprocessed_cow: transfer = self.unprocessed_cow.pop(0) - solver = transfer.receiver + solver = transfer.solver # Remove the element if it exists (assuming it won't have to be reinserted) overdraft = self.overdrafts.pop(solver, None) if overdraft is not None: @@ -121,7 +121,7 @@ def _process_rewards( # Reinsert since there is still an amount owed. self.overdrafts[solver] = overdraft continue - transfer.redirect_to(redirect_map, self.log_saver) + transfer.try_redirect(redirect_map, self.log_saver) self.cow_transfers.append(transfer) # We do not need to worry about any controversy between overdraft # and positive slippage adjustments, because positive/negative slippage @@ -134,7 +134,7 @@ def _process_rewards( ), f"Expected positive slippage got {slippage.amount_wei}" total_positive_slippage += slippage.amount_wei slippage_transfer = Transfer.from_slippage(slippage) - slippage_transfer.redirect_to(redirect_map, self.log_saver) + slippage_transfer.try_redirect(redirect_map, self.log_saver) self.eth_transfers.append(slippage_transfer) return total_positive_slippage diff --git a/src/models/transfer.py b/src/models/transfer.py index 69cf6ea7..98550e01 100644 --- a/src/models/transfer.py +++ b/src/models/transfer.py @@ -35,7 +35,7 @@ def from_transfer(cls, transfer: Transfer) -> CSVTransfer: return cls( token_type=transfer.token_type, token_address=transfer.token.address if transfer.token else None, - receiver=transfer.receiver, + receiver=transfer.recipient, # The primary purpose for this class is to convert amount_wei to amount amount=transfer.amount, ) @@ -46,8 +46,24 @@ class Transfer: """Total amount reimbursed for accounting period""" token: Optional[Token] - receiver: Address + _solver: Address amount_wei: int + _redirect_target: Optional[Address] = None + + def __init__(self, token: Optional[Token], solver: Address, amount_wei: int): + self.token = token + self._solver = solver + self.amount_wei = amount_wei + + @property + def solver(self) -> Address: + """Read access to the solver field""" + return self._solver + + @property + def redirect_target(self) -> Optional[Address]: + """Read access to the redirect_target field""" + return self._redirect_target @classmethod def from_dict(cls, obj: dict[str, str]) -> Transfer: @@ -55,7 +71,7 @@ def from_dict(cls, obj: dict[str, str]) -> Transfer: token_address = obj.get("token_address", None) return cls( token=Token(token_address) if token_address else None, - receiver=Address(obj["receiver"]), + solver=Address(obj["receiver"]), amount_wei=int(obj["amount"]), ) @@ -65,7 +81,7 @@ def from_dataframe(cls, pdf: pd.DataFrame) -> list[Transfer]: return [ cls( token=Token(row["token_address"]) if row["token_address"] else None, - receiver=Address(row["receiver"]), + solver=Address(row["receiver"]), amount_wei=int(row["amount"]), ) for _, row in pdf.iterrows() @@ -85,6 +101,11 @@ def summarize(transfers: list[Transfer]) -> str: f"Total COW Funds needed: {cow_total / 10 ** 18:.4f}\n" ) + @property + def recipient(self) -> Address: + """The correct way to access the true recipient of a transfer""" + return self.redirect_target if self.redirect_target is not None else self.solver + @staticmethod def consolidate(transfer_list: list[Transfer]) -> list[Transfer]: """ @@ -95,14 +116,14 @@ def consolidate(transfer_list: list[Transfer]) -> list[Transfer]: transfer_dict: dict[tuple, Transfer] = {} for transfer in transfer_list: - key = (transfer.receiver, transfer.token) + key = (transfer.recipient, transfer.token) if key in transfer_dict: transfer_dict[key] = transfer_dict[key].merge(transfer) else: transfer_dict[key] = transfer return sorted( transfer_dict.values(), - key=lambda t: (-t.amount, t.receiver, t.token), + key=lambda t: (-t.amount, t.recipient, t.token), ) @property @@ -123,10 +144,10 @@ def amount(self) -> float: def add_slippage(self, slippage: SolverSlippage, log_saver: PrintStore) -> None: """Adds Adjusts Transfer amount by Slippage amount""" - assert self.receiver == slippage.solver_address, "receiver != solver" + assert self.solver == slippage.solver_address, "receiver != solver" adjustment = slippage.amount_wei log_saver.print( - f"Deducting slippage for solver {self.receiver}" + f"Deducting slippage for solver {self.solver}" f"by {adjustment / 10 ** 18:.5f} ({slippage.solver_name})", category=Category.SLIPPAGE, ) @@ -137,30 +158,34 @@ def add_slippage(self, slippage: SolverSlippage, log_saver: PrintStore) -> None: def merge(self, other: Transfer) -> Transfer: """ - Merge two transfers (acts like addition) - if all fields except amount are equal, returns a transfer who amount is the sum + Merge two transfers (acts like addition) if all fields except amount are equal, + returns a transfer who amount is the sum. + Merges incur information loss (particularly in the category of redirects). + Note that two transfers of the same token with different receivers, + but redirected to the same address, will get merged and original receivers will be forgotten """ merge_requirements = [ - self.receiver == other.receiver, + self.recipient == other.recipient, self.token == other.token, ] if all(merge_requirements): return Transfer( token=self.token, - receiver=self.receiver, + solver=self.recipient, amount_wei=self.amount_wei + other.amount_wei, ) raise ValueError( - f"Can't merge tokens {self}, {other}. " + f"Can't merge transfers {self}, {other}. " f"Requirements met {merge_requirements}" ) def as_multisend_tx(self) -> MultiSendTx: """Converts Transfer into encoded MultiSendTx bytes""" + receiver = self.recipient.address if self.token_type == TokenType.NATIVE: return MultiSendTx( operation=MultiSendOperation.CALL, - to=self.receiver.address, + to=receiver, value=self.amount_wei, data=HexStr("0x"), ) @@ -171,31 +196,31 @@ def as_multisend_tx(self) -> MultiSendTx: to=str(self.token.address), value=0, data=erc20().encodeABI( - fn_name="transfer", args=[self.receiver.address, self.amount_wei] + fn_name="transfer", args=[receiver, self.amount_wei] ), ) raise ValueError(f"Unsupported type {self.token_type}") def __str__(self) -> str: if self.token_type == TokenType.NATIVE: - return f"TransferETH(receiver={self.receiver}, amount_wei={self.amount})" + return f"TransferETH(receiver={self.recipient}, amount_wei={self.amount})" if self.token_type == TokenType.ERC20: return ( f"Transfer(" f"token_address={self.token}, " - f"receiver={self.receiver}, " + f"recipient={self.recipient}, " f"amount_wei={self.amount})" ) raise ValueError(f"Invalid Token Type {self.token_type}") - def redirect_to( + def try_redirect( self, redirects: dict[Address, Vouch], log_saver: PrintStore ) -> None: """ Redirects Transfers via Address => Vouch.reward_target This function modifies self! """ - recipient = self.receiver + recipient = self.recipient if recipient in redirects: # Redirect COW rewards to reward target specific by VouchRegistry redirect_address = redirects[recipient].reward_target @@ -205,7 +230,7 @@ def redirect_to( if self.token is None else Category.COW_REDIRECT, ) - self.receiver = redirect_address + self._redirect_target = redirect_address @classmethod def from_slippage(cls, slippage: SolverSlippage) -> Transfer: @@ -215,6 +240,18 @@ def from_slippage(cls, slippage: SolverSlippage) -> Transfer: """ return cls( token=None, - receiver=slippage.solver_address, + solver=slippage.solver_address, amount_wei=slippage.amount_wei, ) + + @staticmethod + def sort_list(transfer_list: list[Transfer]) -> None: + """ + This is the preferred and tested sort order we use for generating the transfer file. + It ensures that transfers are grouped + 1. first by initial recipient (i.e. solvers), + 2. then by token address (with native ETH last) + and finally order by amount descending (so that largest in category occur first). + Note that this method mutates the input data and nothing in returned. + """ + transfer_list.sort(key=lambda t: (t.solver, str(t.token), -t.amount)) diff --git a/src/utils/script_args.py b/src/utils/script_args.py index 58eae752..1f96c193 100644 --- a/src/utils/script_args.py +++ b/src/utils/script_args.py @@ -18,6 +18,7 @@ class ScriptArgs: dune: DuneFetcher post_tx: bool dry_run: bool + consolidate_transfers: bool def generic_script_init(description: str) -> ScriptArgs: @@ -47,6 +48,13 @@ def generic_script_init(description: str) -> ScriptArgs: default=DuneVersion.V2, choices=list(DuneVersion), ) + parser.add_argument( + "--consolidate-transfers", + type=bool, + help="Flag to indicate whether payout transfer file should be optimized " + "(i.e. squash transfers having same receiver-token pair) ", + default=False, + ) parser.add_argument( "--dry-run", type=bool, @@ -64,4 +72,5 @@ def generic_script_init(description: str) -> ScriptArgs: ), post_tx=args.post_tx, dry_run=args.dry_run, + consolidate_transfers=args.consolidate_transfers, ) diff --git a/tests/e2e/test_post_transaction.py b/tests/e2e/test_post_transaction.py index 683083a4..4bed0807 100644 --- a/tests/e2e/test_post_transaction.py +++ b/tests/e2e/test_post_transaction.py @@ -26,9 +26,9 @@ def test_token_decimals(self): token_transfer = Transfer( token=self.cow, amount_wei=15, - receiver=self.receiver, + solver=self.receiver, ) - native_transfer = Transfer(token=None, receiver=self.receiver, amount_wei=2) + native_transfer = Transfer(token=None, solver=self.receiver, amount_wei=2) post_multisend( safe_address=Web3().toChecksumAddress(self.test_safe), diff --git a/tests/e2e/test_transfer_file.py b/tests/e2e/test_transfer_file.py index 04f1a49e..85ebb41e 100644 --- a/tests/e2e/test_transfer_file.py +++ b/tests/e2e/test_transfer_file.py @@ -24,12 +24,12 @@ def test_process_transfers(self): mixed_transfers = [ Transfer( token=None, - receiver=barn_zerox, + solver=barn_zerox, amount_wei=185360274773133130, ), - Transfer(token=None, receiver=other_solver, amount_wei=1 * ONE_ETH), - Transfer(token=cow_token, receiver=barn_zerox, amount_wei=600 * ONE_ETH), - Transfer(token=cow_token, receiver=other_solver, amount_wei=2000 * ONE_ETH), + Transfer(token=None, solver=other_solver, amount_wei=1 * ONE_ETH), + Transfer(token=cow_token, solver=barn_zerox, amount_wei=600 * ONE_ETH), + Transfer(token=cow_token, solver=other_solver, amount_wei=2000 * ONE_ETH), ] slippages = SplitSlippages.from_data_set( [ @@ -58,7 +58,7 @@ def test_process_transfers(self): [ Transfer( token=cow_token, - receiver=other_solver, + solver=other_solver, amount_wei=845094377028141056000, ) ], diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 546dfa10..1c65a5e0 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -11,9 +11,11 @@ from src.fetch.transfer_file import Transfer from src.models.accounting_period import AccountingPeriod from src.models.token import Token +from src.models.vouch import Vouch from src.utils.print_store import PrintStore from tests.queries.test_internal_trades import TransferType +from tests.unit.util_methods import redirected_transfer ONE_ETH = 10**18 @@ -49,7 +51,7 @@ def test_add_slippage(self): solver = Address.zero() transfer = Transfer( token=None, - receiver=solver, + solver=solver, amount_wei=ONE_ETH, ) positive_slippage = SolverSlippage( @@ -77,7 +79,19 @@ def test_add_slippage(self): f"by {overdraft_slippage.amount_wei / 10**18}", ) - def test_consolidation(self): + def test_recipient(self): + receiver = Address.from_int(1) + transfer = Transfer( + token=None, + solver=Address.from_int(1), + amount_wei=0, + ) + self.assertEqual(transfer.recipient, receiver) + redirect = Address.from_int(2) + transfer._redirect_target = redirect + self.assertEqual(transfer.recipient, redirect) + + def test_basic_consolidation(self): recipients = [ Address.from_int(0), Address.from_int(1), @@ -89,42 +103,42 @@ def test_consolidation(self): transfer_list = [ Transfer( token=tokens[0], - receiver=recipients[0], + solver=recipients[0], amount_wei=1 * ONE_ETH, ), Transfer( token=tokens[0], - receiver=recipients[0], + solver=recipients[0], amount_wei=2 * ONE_ETH, ), Transfer( token=tokens[1], - receiver=recipients[0], + solver=recipients[0], amount_wei=3 * ONE_ETH, ), Transfer( token=tokens[0], - receiver=recipients[1], + solver=recipients[1], amount_wei=4 * ONE_ETH, ), Transfer( token=None, - receiver=recipients[0], + solver=recipients[0], amount_wei=5 * ONE_ETH, ), Transfer( token=None, - receiver=recipients[0], + solver=recipients[0], amount_wei=6 * ONE_ETH, ), Transfer( token=None, - receiver=recipients[1], + solver=recipients[1], amount_wei=7 * ONE_ETH, ), Transfer( token=None, - receiver=recipients[1], + solver=recipients[1], amount_wei=8 * ONE_ETH, ), ] @@ -132,69 +146,139 @@ def test_consolidation(self): expected = [ Transfer( token=None, - receiver=recipients[1], + solver=recipients[1], amount_wei=15 * ONE_ETH, ), Transfer( token=None, - receiver=recipients[0], + solver=recipients[0], amount_wei=11 * ONE_ETH, ), Transfer( token=tokens[0], - receiver=recipients[1], + solver=recipients[1], amount_wei=4 * ONE_ETH, ), Transfer( token=tokens[0], - receiver=recipients[0], + solver=recipients[0], amount_wei=3 * ONE_ETH, ), Transfer( token=tokens[1], - receiver=recipients[0], + solver=recipients[0], amount_wei=3 * ONE_ETH, ), ] self.assertEqual(expected, Transfer.consolidate(transfer_list)) - def test_merge(self): + def test_consolidation_with_redirect(self): + receiver = Address.from_int(0) + redirect = Address.from_int(1) + + transfer_list = [ + Transfer( + token=None, + solver=receiver, + amount_wei=1 * ONE_ETH, + ), + Transfer( + token=None, + solver=receiver, + amount_wei=2 * ONE_ETH, + ), + redirected_transfer( + token=None, + solver=receiver, + amount_wei=3 * ONE_ETH, + redirect=redirect, + ), + ] + + expected = [ + Transfer( + token=None, + solver=receiver, + amount_wei=3 * ONE_ETH, + ), + redirected_transfer( + token=None, solver=receiver, amount_wei=3 * ONE_ETH, redirect=redirect + ), + ] + results = Transfer.consolidate(transfer_list) + self.assertEqual(expected, results) + + transfer_list = [ + Transfer( + token=None, + solver=receiver, + amount_wei=1 * ONE_ETH, + ), + redirected_transfer( + token=None, + solver=receiver, + amount_wei=2 * ONE_ETH, + redirect=redirect, + ), + redirected_transfer( + token=None, + solver=receiver, + amount_wei=3 * ONE_ETH, + redirect=redirect, + ), + ] + expected = [ + Transfer( + token=None, + solver=redirect, + amount_wei=5 * ONE_ETH, + ), + Transfer( + token=None, + solver=receiver, + amount_wei=1 * ONE_ETH, + ), + ] + results = Transfer.consolidate(transfer_list) + self.assertEqual(expected, results) + + def test_basic_merge_without_redirects(self): receiver = Address.zero() # Native Transfer Merge native_transfer1 = Transfer( token=None, - receiver=receiver, + solver=receiver, amount_wei=ONE_ETH, ) native_transfer2 = Transfer( token=None, - receiver=receiver, + solver=receiver, amount_wei=ONE_ETH, ) self.assertEqual( native_transfer1.merge(native_transfer2), Transfer( token=None, - receiver=receiver, + solver=receiver, amount_wei=2 * ONE_ETH, ), ) # ERC20 Transfer Merge erc20_transfer1 = Transfer( token=self.token_1, - receiver=receiver, + solver=receiver, amount_wei=ONE_ETH, ) erc20_transfer2 = Transfer( token=self.token_1, - receiver=receiver, + solver=receiver, amount_wei=ONE_ETH, ) self.assertEqual( erc20_transfer1.merge(erc20_transfer2), Transfer( token=self.token_1, - receiver=receiver, + solver=receiver, amount_wei=2 * ONE_ETH, ), ) @@ -202,7 +286,7 @@ def test_merge(self): with self.assertRaises(ValueError) as err: native_transfer1.merge(erc20_transfer1) self.assertEqual( - f"Can't merge tokens {native_transfer1}, {erc20_transfer1}. " + f"Can't merge transfers {native_transfer1}, {erc20_transfer1}. " f"Requirements met [True, False]", str(err.exception), ) @@ -211,17 +295,17 @@ def test_merge(self): # Different recipients t1 = Transfer( token=self.token_1, - receiver=Address.from_int(1), + solver=Address.from_int(1), amount_wei=2 * ONE_ETH, ) t2 = Transfer( token=self.token_1, - receiver=Address.from_int(2), + solver=Address.from_int(2), amount_wei=2 * ONE_ETH, ) t1.merge(t2) self.assertEqual( - f"Can't merge tokens {t1}, {t2}. Requirements met [False, True]", + f"Can't merge transfers {t1}, {t2}. Requirements met [False, True]", str(err.exception), ) @@ -229,24 +313,75 @@ def test_merge(self): # Different Token Addresses t1 = Transfer( token=self.token_1, - receiver=receiver, + solver=receiver, amount_wei=2 * ONE_ETH, ) t2 = Transfer( token=self.token_2, - receiver=receiver, + solver=receiver, amount_wei=2 * ONE_ETH, ) t1.merge(t2) self.assertEqual( - f"Can't merge tokens {t1}, {t2}. Requirements met [True, False]", + f"Can't merge transfers {t1}, {t2}. Requirements met [True, False]", + str(err.exception), + ) + + def test_merge_with_redirects(self): + receiver_1 = Address.from_int(1) + receiver_2 = Address.from_int(2) + redirect = Address.from_int(3) + + transfer = redirected_transfer( + token=None, solver=receiver_1, amount_wei=ONE_ETH, redirect=redirect + ) + expected = Transfer( + token=None, + solver=redirect, + amount_wei=2 * ONE_ETH, + ) + self.assertEqual( + transfer.merge( + redirected_transfer( + token=None, + solver=receiver_2, + amount_wei=ONE_ETH, + redirect=redirect, + ) + ), + # Both redirected to same address get merged. + expected, + ) + self.assertEqual( + transfer.merge( + Transfer( + token=None, + solver=redirect, + amount_wei=ONE_ETH, + ) + ), + # one redirected and the other with initial recipient as redirect address get merged. + expected, + ) + + with self.assertRaises(ValueError) as err: + # Fail to merge redirected transfer with one whose recipient + # is the original receiver of the redirected transfer + other = Transfer( + token=None, + solver=transfer.solver, + amount_wei=ONE_ETH, + ) + transfer.merge(other) + self.assertEqual( + f"Can't merge transfers {transfer}, {other}. Requirements met [False, True]", str(err.exception), ) def test_receiver_error(self): transfer = Transfer( token=None, - receiver=Address.from_int(1), + solver=Address.from_int(1), amount_wei=1 * ONE_ETH, ) with self.assertRaises(AssertionError) as err: @@ -272,7 +407,7 @@ def test_from_dict(self): ), Transfer( token=None, - receiver=receiver, + solver=receiver, amount_wei=1234 * 10**15, ), ) @@ -287,7 +422,7 @@ def test_from_dict(self): ), Transfer( token=Token(COW_TOKEN_ADDRESS), - receiver=Address.from_int(1), + solver=Address.from_int(1), amount_wei=1234 * 10**15, ), ) @@ -306,21 +441,21 @@ def test_from_dataframe(self): expected = [ Transfer( token=None, - receiver=receiver, + solver=receiver, amount_wei=12345, ), Transfer( token=Token(COW_TOKEN_ADDRESS), - receiver=receiver, + solver=receiver, amount_wei=678910, ), ] self.assertEqual(expected, Transfer.from_dataframe(transfer_df)) - def test_as_multisend_tx(self): + def test_basic_as_multisend_tx(self): receiver = Address("0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1") - native_transfer = Transfer(token=None, receiver=receiver, amount_wei=16) + native_transfer = Transfer(token=None, solver=receiver, amount_wei=16) self.assertEqual( native_transfer.as_multisend_tx(), MultiSendTx( @@ -332,7 +467,7 @@ def test_as_multisend_tx(self): ) erc20_transfer = Transfer( token=Token(COW_TOKEN_ADDRESS), - receiver=receiver, + solver=receiver, amount_wei=15, ) self.assertEqual( @@ -345,16 +480,49 @@ def test_as_multisend_tx(self): ), ) + def test_as_multisend_tx_with_redirects(self): + receiver = Address.from_int(1) + redirect = Address.from_int(2) + native_transfer = redirected_transfer( + token=None, solver=receiver, amount_wei=16, redirect=redirect + ) + self.assertEqual( + native_transfer.as_multisend_tx(), + MultiSendTx( + operation=MultiSendOperation.CALL, + to=redirect.address, + value=16, + data=HexStr("0x"), + ), + ) + token_address = Address.from_int(1) + token = Token(address=token_address, decimals=10) + erc20_transfer = redirected_transfer( + token=token, + solver=receiver, + amount_wei=15, + redirect=redirect, + ) + self.assertEqual( + erc20_transfer.as_multisend_tx(), + MultiSendTx( + operation=MultiSendOperation.CALL, + to=token_address.address, + value=0, + data=erc20().encodeABI(fn_name="transfer", args=[redirect.address, 15]), + ), + ) + def test_summarize(self): receiver = Address.from_int(1) eth_amount = 123456789101112131415 cow_amount = 9999999999999999999999999 result = Transfer.summarize( [ - Transfer(token=None, receiver=receiver, amount_wei=eth_amount), + Transfer(token=None, solver=receiver, amount_wei=eth_amount), Transfer( token=Token(COW_TOKEN_ADDRESS), - receiver=receiver, + solver=receiver, amount_wei=cow_amount, ), ] @@ -364,6 +532,107 @@ def test_summarize(self): "Total ETH Funds needed: 123.4568\nTotal COW Funds needed: 10000000.0000\n", ) + def test_try_redirect(self): + """ + Test demonstrates that try_redirect works as expected for our use case. + However, it also demonstrates how bad it is to pass in an unstructured hashmap + that expects the keys to be equal the solver field of its values! + TODO - fix this strange error prone issue! + """ + dummy_print_store = PrintStore() + receiver = Address.from_int(1) + redirect = Address.from_int(2) + # Try redirect elsewhere + t1 = Transfer(token=None, amount_wei=1, solver=receiver) + vouch_forward = Vouch( + bonding_pool=Address.zero(), reward_target=redirect, solver=receiver + ) + t1.try_redirect({vouch_forward.solver: vouch_forward}, dummy_print_store) + self.assertEqual(t1.recipient, redirect) + + vouch_reverse = Vouch( + bonding_pool=Address.zero(), reward_target=receiver, solver=redirect + ) + # Redirect back! + t1.try_redirect({vouch_reverse.solver: vouch_reverse}, dummy_print_store) + self.assertEqual(t1.recipient, receiver) + + # no action redirect. + another_address = Address.from_int(5) + t2 = Transfer(token=None, amount_wei=1, solver=another_address) + disjoint_redirect_map = { + vouch_forward.solver: vouch_forward, + vouch_reverse.solver: vouch_reverse, + } + # This assertion implies we should expect t2 to remain unchanged after "try_redirect" + self.assertFalse(t2.recipient in disjoint_redirect_map.keys()) + t2.try_redirect(disjoint_redirect_map, dummy_print_store) + self.assertEqual(t2, Transfer(token=None, amount_wei=1, solver=another_address)) + + def test_sorted_output(self): + solver_1 = Address.from_int(1) + solver_2 = Address.from_int(2) + solver_3 = Address.from_int(3) + + reward_target_1 = Address.from_int(4) + reward_target_2 = Address.from_int(4) + + # These are not redirected. + eth_1 = Transfer(token=None, solver=solver_1, amount_wei=2) + eth_2 = Transfer(token=None, solver=solver_2, amount_wei=3) + eth_3 = Transfer(token=None, solver=solver_3, amount_wei=5) + execution_reimbursements = [eth_1, eth_2, eth_3] + cow_token = Token(COW_TOKEN_ADDRESS) + cow_1 = redirected_transfer( + token=cow_token, + solver=solver_1, + amount_wei=10, + redirect=reward_target_1, + ) + cow_2 = redirected_transfer( + token=cow_token, + solver=solver_2, + amount_wei=20, + redirect=reward_target_2, + ) + cow_3 = redirected_transfer( + token=cow_token, + solver=solver_3, + amount_wei=30, + redirect=reward_target_1, + ) + cow_rewards = [cow_1, cow_2, cow_3] + slip_1 = redirected_transfer( + token=None, solver=solver_1, amount_wei=1, redirect=reward_target_2 + ) + slip_2 = redirected_transfer( + token=Token(COW_TOKEN_ADDRESS), + solver=solver_2, + amount_wei=4, + redirect=reward_target_2, + ) + positive_slippage = [slip_1, slip_2] + + payout_transfers = execution_reimbursements + cow_rewards + positive_slippage + Transfer.sort_list(payout_transfers) + # This demonstrates that the sorting technique groups solvers (i.e. original recipients before redirects first) + # Then by token (with NATIVE ETH Last Since "0x" < "None") + # and finally by amount descending. + # Note that eth_1.amount > slip_1.amount while eth_2.amount < slip_2.amount + self.assertEqual( + payout_transfers, + [ + cow_1, + eth_1, + slip_1, # Solver 1 Transfers + cow_2, + slip_2, + eth_2, # Solver 2 Transfers + cow_3, + eth_3, # Solver 3 Transfers + ], + ) + class TestAccountingPeriod(unittest.TestCase): def test_str(self): diff --git a/tests/unit/test_multisend.py b/tests/unit/test_multisend.py index df9fccb8..7974b104 100644 --- a/tests/unit/test_multisend.py +++ b/tests/unit/test_multisend.py @@ -23,7 +23,7 @@ def test_prepend_unwrap(self): "0xA03be496e67Ec29bC62F01a428683D7F9c204930" ) big_native_transfer = Transfer( - token=None, receiver=Address.zero(), amount_wei=many_eth + token=None, solver=Address.zero(), amount_wei=many_eth ).as_multisend_tx() with self.assertRaises(ValueError): @@ -41,7 +41,7 @@ def test_prepend_unwrap(self): transactions = [ Transfer( token=None, - receiver=Address.zero(), + solver=Address.zero(), amount_wei=eth_balance + 1, # More ETH than account has! ).as_multisend_tx() ] @@ -73,7 +73,7 @@ def test_multisend_encoding(self): ) native_transfer = Transfer( - token=None, receiver=receiver, amount_wei=16 + token=None, solver=receiver, amount_wei=16 ).as_multisend_tx() self.assertEqual( build_encoded_multisend([native_transfer], client=self.client), @@ -86,7 +86,7 @@ def test_multisend_encoding(self): ) erc20_transfer = Transfer( token=cow_token, - receiver=receiver, + solver=receiver, amount_wei=15, ).as_multisend_tx() self.assertEqual( diff --git a/tests/unit/test_split_transfer.py b/tests/unit/test_split_transfer.py index 52395b1a..7be876e5 100644 --- a/tests/unit/test_split_transfer.py +++ b/tests/unit/test_split_transfer.py @@ -11,6 +11,7 @@ from src.models.transfer import Transfer from src.models.vouch import Vouch from src.utils.print_store import PrintStore +from tests.unit.util_methods import redirected_transfer ONE_ETH = 10**18 @@ -40,15 +41,13 @@ def construct_split_transfers_and_process( eth_transfers = [ Transfer( token=None, - receiver=solvers[i], + solver=solvers[i], amount_wei=eth_amounts[i], ) for i in range(len(solvers)) ] cow_transfers = [ - Transfer( - token=self.cow_token, receiver=solvers[i], amount_wei=cow_rewards[i] - ) + Transfer(token=self.cow_token, solver=solvers[i], amount_wei=cow_rewards[i]) for i in range(len(solvers)) ] accounting = SplitTransfers( @@ -76,11 +75,11 @@ def test_process_native_transfers(self): mixed_transfers = [ Transfer( token=None, - receiver=self.solver, + solver=self.solver, amount_wei=amount_of_transfer, ), Transfer( - token=self.cow_token, receiver=self.solver, amount_wei=600 * ONE_ETH + token=self.cow_token, solver=self.solver, amount_wei=600 * ONE_ETH ), ] @@ -99,7 +98,7 @@ def test_process_native_transfers(self): def test_process_rewards(self): cow_reward = 600 * ONE_ETH mixed_transfers = [ - Transfer(token=self.cow_token, receiver=self.solver, amount_wei=cow_reward), + Transfer(token=self.cow_token, solver=self.solver, amount_wei=cow_reward), ] accounting = SplitTransfers(self.period, mixed_transfers, PrintStore()) reward_target = Address.from_int(7) @@ -121,20 +120,22 @@ def test_process_rewards(self): self.assertEqual( accounting.eth_transfers, [ - Transfer( + redirected_transfer( token=None, - receiver=redirect_map[self.solver].reward_target, + solver=self.solver, amount_wei=slippage_amount, + redirect=redirect_map[self.solver].reward_target, ) ], ) self.assertEqual( accounting.cow_transfers, [ - Transfer( + redirected_transfer( token=self.cow_token, - receiver=redirect_map[self.solver].reward_target, + solver=self.solver, amount_wei=cow_reward, + redirect=redirect_map[self.solver].reward_target, ) ], ) @@ -157,24 +158,26 @@ def test_full_process_with_positive_slippage(self): # The ETH Spent Transfer( token=None, - receiver=self.solver, + solver=self.solver, amount_wei=eth_amount, ), # The redirected positive slippage - Transfer( + redirected_transfer( token=None, - receiver=self.redirect_map[self.solver].reward_target, + solver=self.solver, amount_wei=slippage_amount, + redirect=self.redirect_map[self.solver].reward_target, ), ], ) self.assertEqual( accounting.cow_transfers, [ - Transfer( + redirected_transfer( token=self.cow_token, - receiver=self.redirect_map[self.solver].reward_target, + solver=self.solver, amount_wei=cow_reward, + redirect=self.redirect_map[self.solver].reward_target, ), ], ) @@ -198,7 +201,7 @@ def test_process_with_negative_slippage_not_exceeding_eth(self): [ Transfer( token=None, - receiver=self.solver, + solver=self.solver, # Slippage is negative (so it is added here) amount_wei=eth_amount + slippage_amount, ), @@ -207,10 +210,11 @@ def test_process_with_negative_slippage_not_exceeding_eth(self): self.assertEqual( accounting.cow_transfers, [ - Transfer( + redirected_transfer( token=self.cow_token, - receiver=self.redirect_map[self.solver].reward_target, + solver=self.solver, amount_wei=cow_reward, + redirect=self.redirect_map[self.solver].reward_target, ), ], ) @@ -236,12 +240,13 @@ def test_process_with_overdraft_exceeding_eth_not_cow(self): self.assertEqual( accounting.cow_transfers, [ - Transfer( + redirected_transfer( token=self.cow_token, - receiver=self.redirect_map[self.solver].reward_target, + solver=self.solver, # This is the amount of COW deducted based on a "deterministic" price # on the date of the fixed accounting period. amount_wei=cow_reward - 11549056229718590750720, + redirect=self.redirect_map[self.solver].reward_target, ) ], ) @@ -302,12 +307,12 @@ def test_process_with_missing_redirect(self): [ Transfer( token=None, - receiver=self.solver, + solver=self.solver, amount_wei=eth_amount, ), Transfer( token=None, - receiver=self.solver, + solver=self.solver, amount_wei=slippage_amount, ), ], @@ -317,7 +322,7 @@ def test_process_with_missing_redirect(self): [ Transfer( token=self.cow_token, - receiver=self.solver, + solver=self.solver, amount_wei=cow_reward, ), ], @@ -365,12 +370,12 @@ def test_process_multiple_solver_same_reward_target(self): [ Transfer( token=None, - receiver=solvers[0], + solver=solvers[0], amount_wei=eth_amounts[0], ), Transfer( token=None, - receiver=solvers[1], + solver=solvers[1], amount_wei=eth_amounts[1], ), ], @@ -378,15 +383,17 @@ def test_process_multiple_solver_same_reward_target(self): self.assertEqual( accounting.cow_transfers, [ - Transfer( + redirected_transfer( token=self.cow_token, - receiver=reward_target, + solver=solvers[0], amount_wei=cow_rewards[0], + redirect=reward_target, ), - Transfer( + redirected_transfer( token=self.cow_token, - receiver=reward_target, + solver=solvers[1], amount_wei=cow_rewards[1], + redirect=reward_target, ), ], ) diff --git a/tests/unit/util_methods.py b/tests/unit/util_methods.py new file mode 100644 index 00000000..da2e5138 --- /dev/null +++ b/tests/unit/util_methods.py @@ -0,0 +1,10 @@ +from src.models.transfer import Transfer + + +def redirected_transfer(token, solver, amount_wei, redirect) -> Transfer: + # simple way to set up a transfer object with non-empty + # redirect target field (for testing) + transfer = Transfer(token, solver, amount_wei) + + transfer._redirect_target = redirect + return transfer