-
Notifications
You must be signed in to change notification settings - Fork 57
Open
Labels
help wantedExtra attention is neededExtra attention is needed
Description
We are currently working on the per wallet implementation PR #138
Since there are a lot of complicated issues in the two threads that already exist, I thought I would post something separate here. I wrote this test:
# Test for transfer with crypto fee spanning multiple lots with FIFO
_Test(
description="Transfer with crypto fee spanning multiple lots (FIFO)",
input=[
InTransactionDescriptor("1", 1, 1, "Coinbase", "Bob", 100, 5),
InTransactionDescriptor("2", 2, 2, "Coinbase", "Bob", 110, 3),
# Send 4 BTC with 3 BTC fee = 7 total, spans both lots with FIFO
IntraTransactionDescriptor("3", 3, 3, "Coinbase", "Bob", "Kraken", "Bob", 120, 7, 4),
],
want_per_wallet_transactions={
Account("Coinbase", "Bob"): [
InTransactionDescriptor("1", 1, 1, "Coinbase", "Bob", 100, 5,
to_lot_unique_ids={Account("Kraken", "Bob"): ["3/-1"]}),
InTransactionDescriptor("2", 2, 2, "Coinbase", "Bob", 110, 3,
to_lot_unique_ids={Account("Kraken", "Bob"): ["3/-2"]}),
IntraTransactionDescriptor("3", 3, 3, "Coinbase", "Bob", "Kraken", "Bob", 120, 7, 4),
],
Account("Kraken", "Bob"): [
InTransactionDescriptor("3/-1", 3, -1, "Kraken", "Bob", 100, 4,
from_lot_unique_id="1", cost_basis_day=1),
# Note: With fee, only 4 BTC received but 7 BTC sent (5 from lot 1, 2 from lot 2)
# FIFO takes from lot 1 first for the 4 BTC received
],
},
want_amounts={
Account(exchange='Coinbase', holder='Bob'): {'1': 0, '2': 1},
Account(exchange='Kraken', holder='Bob'): {},
},
want_error="",
), It fails because the current resolver attempts to take the fee out of the last lot only and doesn't account for splitting across lots. As you can see in the following lines:
rp2/src/rp2/transfer_analyzer.py
Lines 290 to 321 in 046dfe9
| while True: | |
| current_in_lot_and_amount = self.__transfer_semantics.seek_non_exhausted_acquired_lot( | |
| from_per_wallet_transactions.in_transactions, transaction.crypto_received | |
| ) | |
| if current_in_lot_and_amount is None: | |
| raise RP2ValueError( | |
| f"Insufficient balance on {from_account} to send funds (amount {amount_left_to_transfer + fee} {transaction.asset}): {transaction}" | |
| ) | |
| original_actual_amounts[current_in_lot_and_amount.acquired_lot] = current_in_lot_and_amount.amount | |
| if current_in_lot_and_amount.amount >= amount_left_to_transfer + fee: | |
| # Pay the fee only in the last lot. | |
| self._process_remaining_transfer_amount( | |
| wallet_2_per_wallet_transactions, | |
| current_in_lot_and_amount, | |
| transaction, | |
| amount_left_to_transfer, | |
| fee, | |
| ) | |
| if transaction.is_self_transfer(): | |
| from_per_wallet_transactions.in_transactions.reset_partial_amounts(self.__transfer_semantics, original_actual_amounts) | |
| break | |
| self._process_remaining_transfer_amount( | |
| wallet_2_per_wallet_transactions, | |
| current_in_lot_and_amount, | |
| transaction, | |
| current_in_lot_and_amount.amount, | |
| ZERO, | |
| ) | |
| amount_left_to_transfer -= current_in_lot_and_amount.amount | |
| else: | |
| raise RP2ValueError(f"Internal error: invalid transaction class: {transaction}") | |
I think this can be fixed by tracking both the remaining amount of the transfer and remaining amount of the fee. I tried to fix it, but I might not have the time. If anyone could help it would be much appreciated.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
help wantedExtra attention is neededExtra attention is needed