Skip to content

Commit

Permalink
[Easy] Remove option to consolidate transfers (#416)
Browse files Browse the repository at this point in the history
This PR removes a command line argument to merge transfers and all code
associated to it.

With our current code, switching on this option would result in
transfers merging transfers which have not been merged on purpose.
Examples for this are transfers for quote and batch rewards, as well as
transfers for protocol fees and partner fees. Removing the option makes
it more difficult to use the script in a wrong way.

At the end of the restructuring of the solver accounting, we might also
want to merge transfers again. This will, however, not be compatible
with the simple logic currently implemented. Thus, keeping the current
code has little benefits.

Removing the command line option allows to also remove the associated
code and tests. This makes the rest of the code easier to change.

This change is a breaking change of the command line API. We currently
do not use the option anywhere, so the change should not affect current
operations.
  • Loading branch information
fhenneke authored Nov 6, 2024
1 parent 9aef57f commit b6b9927
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 427 deletions.
3 changes: 0 additions & 3 deletions src/fetch/transfer_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ def auto_propose(
if tr.amount_wei >= args.min_transfer_amount_cow_atoms:
payout_transfers.append(tr)

if args.consolidate_transfers:
payout_transfers = Transfer.consolidate(payout_transfers)

if args.post_tx:
ssl_context = ssl.create_default_context(cafile=certifi.where())
ssl_context.verify_mode = ssl.CERT_REQUIRED
Expand Down
67 changes: 0 additions & 67 deletions src/models/transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
from dataclasses import dataclass
from typing import Optional

import pandas as pd

from dune_client.types import Address
from eth_typing.encoding import HexStr
from gnosis.safe.multi_send import MultiSendOperation, MultiSendTx
Expand Down Expand Up @@ -58,28 +56,6 @@ def __init__(self, token: Optional[Token], recipient: Address, amount_wei: int):
self._recipient = recipient
self.amount_wei = amount_wei

@classmethod
def from_dict(cls, obj: dict[str, str]) -> Transfer:
"""Converts Dune data dict to object with types"""
token_address = obj.get("token_address", None)
return cls(
token=Token(token_address) if token_address else None,
recipient=Address(obj["receiver"]),
amount_wei=int(obj["amount"]),
)

@classmethod
def from_dataframe(cls, pdf: pd.DataFrame) -> list[Transfer]:
"""Converts Pandas Dataframe into list of Transfers"""
return [
cls(
token=Token(row["token_address"]) if row["token_address"] else None,
recipient=Address(row["receiver"]),
amount_wei=int(row["amount"]),
)
for _, row in pdf.iterrows()
]

@staticmethod
def summarize(transfers: list[Transfer]) -> str:
"""Summarizes transfers with totals"""
Expand All @@ -99,26 +75,6 @@ def recipient(self) -> Address:
"""Read access to the recipient of a transfer"""
return self._recipient

@staticmethod
def consolidate(transfer_list: list[Transfer]) -> list[Transfer]:
"""
Removes redundancy of a transfer list by consolidating _duplicate transfers_.
Duplicates defined as transferring the same token to one recipient multiple times.
This optimizes gas cost of multiple transfers.
"""

transfer_dict: dict[tuple, Transfer] = {}
for transfer in transfer_list:
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.recipient, t.token),
)

@property
def token_type(self) -> TokenType:
"""Returns the type of transfer (Native or ERC20)"""
Expand All @@ -135,29 +91,6 @@ def amount(self) -> float:
assert self.token is not None
return self.amount_wei / int(10**self.token.decimals)

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.
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.recipient == other.recipient,
self.token == other.token,
]
if all(merge_requirements):
return Transfer(
token=self.token,
recipient=self.recipient,
amount_wei=self.amount_wei + other.amount_wei,
)
raise ValueError(
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 = Web3.to_checksum_address(self.recipient.address)
Expand Down
9 changes: 0 additions & 9 deletions src/utils/script_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class ScriptArgs:
dune: DuneFetcher
post_tx: bool
dry_run: bool
consolidate_transfers: bool
min_transfer_amount_wei: int
min_transfer_amount_cow_atoms: int
ignore_slippage: bool
Expand All @@ -44,13 +43,6 @@ def generic_script_init(description: str) -> ScriptArgs:
"(requires valid env var `PROPOSER_PK`)",
default=False,
)
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,
Expand Down Expand Up @@ -85,7 +77,6 @@ def generic_script_init(description: str) -> ScriptArgs:
),
post_tx=args.post_tx,
dry_run=args.dry_run,
consolidate_transfers=args.consolidate_transfers,
min_transfer_amount_wei=args.min_transfer_amount_wei,
min_transfer_amount_cow_atoms=args.min_transfer_amount_cow_atoms,
ignore_slippage=args.ignore_slippage,
Expand Down
Loading

0 comments on commit b6b9927

Please sign in to comment.