From b61e26f028d215d7d55b40d98fb4d223b6c54bec Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Sun, 5 May 2024 23:22:11 -0700 Subject: [PATCH 1/6] Make FIFO accounting_method linear time --- src/rp2/abstract_accounting_method.py | 20 ++++++++--- src/rp2/accounting_engine.py | 42 +++++++++++++++++------- src/rp2/plugin/accounting_method/fifo.py | 9 ++--- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/src/rp2/abstract_accounting_method.py b/src/rp2/abstract_accounting_method.py index d3e7c80..3586a79 100644 --- a/src/rp2/abstract_accounting_method.py +++ b/src/rp2/abstract_accounting_method.py @@ -38,13 +38,23 @@ def __init__( accounting_method: "AbstractAccountingMethod", acquired_lot_list: List[InTransaction], acquired_lot_2_partial_amount: Dict[InTransaction, RP2Decimal], - up_to_index: int, ) -> None: self.__accounting_method: AbstractAccountingMethod = accounting_method self.__acquired_lot_list = acquired_lot_list self.__acquired_lot_2_partial_amount = acquired_lot_2_partial_amount + self.__up_to_index = 0 + self.__from_index = 0 + + def set_up_to_index(self, up_to_index: int) -> None: self.__up_to_index = up_to_index + def set_from_index(self, from_index: int) -> None: + self.__from_index = from_index + + @property + def from_index(self) -> int: + return self.__from_index + def has_partial_amount(self, acquired_lot: InTransaction) -> bool: return acquired_lot in self.__acquired_lot_2_partial_amount @@ -60,14 +70,14 @@ def clear_partial_amount(self, acquired_lot: InTransaction) -> None: self.set_partial_amount(acquired_lot, ZERO) def __iter__(self) -> "AccountingMethodIterator": - return AccountingMethodIterator(self.__acquired_lot_list, self.__up_to_index, self.__accounting_method.lot_candidates_order()) + return AccountingMethodIterator(self.__acquired_lot_list, self.__from_index, self.__up_to_index, self.__accounting_method.lot_candidates_order()) class AccountingMethodIterator: - def __init__(self, acquired_lot_list: List[InTransaction], up_to_index: int, order_type: AcquiredLotCandidatesOrder) -> None: + def __init__(self, acquired_lot_list: List[InTransaction], from_index: int, up_to_index: int, order_type: AcquiredLotCandidatesOrder) -> None: self.__acquired_lot_list = acquired_lot_list - self.__start_index = 0 if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else up_to_index - self.__end_index = up_to_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else 0 + self.__start_index = from_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else up_to_index + self.__end_index = up_to_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else from_index self.__step = 1 if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else -1 self.__index = self.__start_index self.__order_type = order_type diff --git a/src/rp2/accounting_engine.py b/src/rp2/accounting_engine.py index 279305e..c84f23c 100644 --- a/src/rp2/accounting_engine.py +++ b/src/rp2/accounting_engine.py @@ -84,6 +84,7 @@ def type_check(cls, name: str, instance: "AccountingEngine") -> "AccountingEngin def __init__(self, years_2_methods: AVLTree[int, AbstractAccountingMethod]) -> None: self.__years_2_methods: AVLTree[int, AbstractAccountingMethod] = years_2_methods + self.__years_2_lot_candidates: AVLTree[int, AcquiredLotCandidates] = AVLTree() if not self.__years_2_methods: raise RP2RuntimeError("Internal error: no accounting method defined") @@ -114,6 +115,22 @@ def initialize( if not self.__acquired_lot_avl.root: raise RP2RuntimeError("Internal error: AVL tree has no root node") + to_visit = [] + node = self.__years_2_methods.root + while node is not None: + self.__years_2_lot_candidates.insert_node( + node.key, AcquiredLotCandidates(node.value, self.__acquired_lot_list, self.__acquired_lot_2_partial_amount) + ) + if node.left: + to_visit.append(node.left) + if node.right: + to_visit.append(node.right) + + if len(to_visit) > 0: + node = to_visit.pop() + else: + break + # AVL tree node keys have this format: _. The internal_id part is needed to disambiguate transactions # that have the same timestamp. Timestamp is in format "YYYYmmddHHMMSS.ffffff" and internal_id is padded right in a string of fixed # length (KEY_DISAMBIGUATOR_LENGTH). @@ -189,18 +206,19 @@ def get_acquired_lot_for_taxable_event( if avl_result.acquired_lot != self.__acquired_lot_list[avl_result.index]: raise RP2RuntimeError("Internal error: acquired_lot incongruence in accounting logic") method = self._get_accounting_method(taxable_event.timestamp.year) - lot_candidates: AcquiredLotCandidates = AcquiredLotCandidates( - method, self.__acquired_lot_list, self.__acquired_lot_2_partial_amount, avl_result.index - ) - acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( - lot_candidates, taxable_event, new_taxable_event_amount - ) - if acquired_lot_and_amount: - return TaxableEventAndAcquiredLot( - taxable_event=taxable_event, - acquired_lot=acquired_lot_and_amount.acquired_lot, - taxable_event_amount=new_taxable_event_amount, - acquired_lot_amount=acquired_lot_and_amount.amount, + lot_candidates: Optional[AcquiredLotCandidates] = self.__years_2_lot_candidates.find_max_value_less_than(taxable_event.timestamp.year) + + if lot_candidates: + lot_candidates.set_up_to_index(avl_result.index) + acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( + lot_candidates, taxable_event, new_taxable_event_amount ) + if acquired_lot_and_amount: + return TaxableEventAndAcquiredLot( + taxable_event=taxable_event, + acquired_lot=acquired_lot_and_amount.acquired_lot, + taxable_event_amount=new_taxable_event_amount, + acquired_lot_amount=acquired_lot_and_amount.amount, + ) raise AcquiredLotsExhaustedException() diff --git a/src/rp2/plugin/accounting_method/fifo.py b/src/rp2/plugin/accounting_method/fifo.py index eb248d3..62af5fa 100644 --- a/src/rp2/plugin/accounting_method/fifo.py +++ b/src/rp2/plugin/accounting_method/fifo.py @@ -25,7 +25,7 @@ from rp2.rp2_decimal import ZERO, RP2Decimal -# FIFO plugin. See https://www.investopedia.com/terms/l/fifo.asp. This plugin uses universal application, not per-wallet application: +# FIFO (First In, First Out) plugin. See https://www.investopedia.com/terms/l/fifo.asp. This plugin uses universal application, not per-wallet application: # this means there is one queue for each coin across every wallet and exchange and the accounting method is applied to each such queue. # More on this at https://www.forbes.com/sites/shehanchandrasekera/2020/09/17/what-crypto-taxpayers-need-to-know-about-fifo-lifo-hifo-specific-id/ class AccountingMethod(AbstractAccountingMethod): @@ -38,8 +38,8 @@ def seek_non_exhausted_acquired_lot( selected_acquired_lot_amount: RP2Decimal = ZERO selected_acquired_lot: Optional[InTransaction] = None acquired_lot: InTransaction - # This loop causes O(m*n) complexity, where m is the number of acquired lots and n in the number of taxable events. The taxable - # event loop is in the caller. Non-trivial optimizations are possible using different data structures but they need to be researched. + # This loop avoids O(m*n) complexity by keeping track of the index of the most recently exhausted lot. + # As FIFO ensures no non-exhausted lots can exist to the left of this index, this approach is O(n). for acquired_lot in lot_candidates: acquired_lot_amount: RP2Decimal = ZERO @@ -48,7 +48,8 @@ def seek_non_exhausted_acquired_lot( elif lot_candidates.get_partial_amount(acquired_lot) > ZERO: acquired_lot_amount = lot_candidates.get_partial_amount(acquired_lot) else: - # The acquired lot has zero partial amount + # The acquired lot has zero partial amount, so we can advance our start offset + lot_candidates.set_from_index(lot_candidates.from_index + 1) continue selected_acquired_lot_amount = acquired_lot_amount From 0e1a5ad7ad7b007576f104718aa875add04f9d9f Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Wed, 29 May 2024 22:12:36 -0700 Subject: [PATCH 2/6] Rename avl_result to acquired_lot_and_index in accounting_engine --- src/rp2/accounting_engine.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rp2/accounting_engine.py b/src/rp2/accounting_engine.py index c84f23c..0d4a01f 100644 --- a/src/rp2/accounting_engine.py +++ b/src/rp2/accounting_engine.py @@ -199,17 +199,17 @@ def get_acquired_lot_for_taxable_event( acquired_lot_amount: RP2Decimal, ) -> TaxableEventAndAcquiredLot: new_taxable_event_amount: RP2Decimal = taxable_event_amount - acquired_lot_amount - avl_result: Optional[_AcquiredLotAndIndex] = self.__acquired_lot_avl.find_max_value_less_than( + acquired_lot_and_index: Optional[_AcquiredLotAndIndex] = self.__acquired_lot_avl.find_max_value_less_than( self._get_avl_node_key_with_max_disambiguator(taxable_event.timestamp) ) - if avl_result is not None: - if avl_result.acquired_lot != self.__acquired_lot_list[avl_result.index]: + if acquired_lot_and_index is not None: + if acquired_lot_and_index.acquired_lot != self.__acquired_lot_list[acquired_lot_and_index.index]: raise RP2RuntimeError("Internal error: acquired_lot incongruence in accounting logic") method = self._get_accounting_method(taxable_event.timestamp.year) lot_candidates: Optional[AcquiredLotCandidates] = self.__years_2_lot_candidates.find_max_value_less_than(taxable_event.timestamp.year) if lot_candidates: - lot_candidates.set_up_to_index(avl_result.index) + lot_candidates.set_up_to_index(acquired_lot_and_index.index) acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( lot_candidates, taxable_event, new_taxable_event_amount ) From 4d3a34b044ec49fe50d063f8d902d7daefc5e1f2 Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Wed, 29 May 2024 22:14:37 -0700 Subject: [PATCH 3/6] Rename up_to_index to to_index --- src/rp2/abstract_accounting_method.py | 14 +++++++------- src/rp2/accounting_engine.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rp2/abstract_accounting_method.py b/src/rp2/abstract_accounting_method.py index 3586a79..bb1f2cd 100644 --- a/src/rp2/abstract_accounting_method.py +++ b/src/rp2/abstract_accounting_method.py @@ -42,11 +42,11 @@ def __init__( self.__accounting_method: AbstractAccountingMethod = accounting_method self.__acquired_lot_list = acquired_lot_list self.__acquired_lot_2_partial_amount = acquired_lot_2_partial_amount - self.__up_to_index = 0 + self.__to_index = 0 self.__from_index = 0 - def set_up_to_index(self, up_to_index: int) -> None: - self.__up_to_index = up_to_index + def set_to_index(self, to_index: int) -> None: + self.__to_index = to_index def set_from_index(self, from_index: int) -> None: self.__from_index = from_index @@ -70,14 +70,14 @@ def clear_partial_amount(self, acquired_lot: InTransaction) -> None: self.set_partial_amount(acquired_lot, ZERO) def __iter__(self) -> "AccountingMethodIterator": - return AccountingMethodIterator(self.__acquired_lot_list, self.__from_index, self.__up_to_index, self.__accounting_method.lot_candidates_order()) + return AccountingMethodIterator(self.__acquired_lot_list, self.__from_index, self.__to_index, self.__accounting_method.lot_candidates_order()) class AccountingMethodIterator: - def __init__(self, acquired_lot_list: List[InTransaction], from_index: int, up_to_index: int, order_type: AcquiredLotCandidatesOrder) -> None: + def __init__(self, acquired_lot_list: List[InTransaction], from_index: int, to_index: int, order_type: AcquiredLotCandidatesOrder) -> None: self.__acquired_lot_list = acquired_lot_list - self.__start_index = from_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else up_to_index - self.__end_index = up_to_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else from_index + self.__start_index = from_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else to_index + self.__end_index = to_index if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else from_index self.__step = 1 if order_type == AcquiredLotCandidatesOrder.OLDER_TO_NEWER else -1 self.__index = self.__start_index self.__order_type = order_type diff --git a/src/rp2/accounting_engine.py b/src/rp2/accounting_engine.py index 0d4a01f..3e3357c 100644 --- a/src/rp2/accounting_engine.py +++ b/src/rp2/accounting_engine.py @@ -209,7 +209,7 @@ def get_acquired_lot_for_taxable_event( lot_candidates: Optional[AcquiredLotCandidates] = self.__years_2_lot_candidates.find_max_value_less_than(taxable_event.timestamp.year) if lot_candidates: - lot_candidates.set_up_to_index(acquired_lot_and_index.index) + lot_candidates.set_to_index(acquired_lot_and_index.index) acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( lot_candidates, taxable_event, new_taxable_event_amount ) From 2ff9021114d72d54f1506d4e50eb059d09ae032b Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Wed, 29 May 2024 22:18:03 -0700 Subject: [PATCH 4/6] Update FIFO comment to reflect linear time complexity --- src/rp2/plugin/accounting_method/fifo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rp2/plugin/accounting_method/fifo.py b/src/rp2/plugin/accounting_method/fifo.py index 62af5fa..6f8ab93 100644 --- a/src/rp2/plugin/accounting_method/fifo.py +++ b/src/rp2/plugin/accounting_method/fifo.py @@ -38,7 +38,7 @@ def seek_non_exhausted_acquired_lot( selected_acquired_lot_amount: RP2Decimal = ZERO selected_acquired_lot: Optional[InTransaction] = None acquired_lot: InTransaction - # This loop avoids O(m*n) complexity by keeping track of the index of the most recently exhausted lot. + # The FIFO plugin features linear complexity by setting lot_candidates from_index to the first non-exhausted lot (to_index is set in the caller). # As FIFO ensures no non-exhausted lots can exist to the left of this index, this approach is O(n). for acquired_lot in lot_candidates: acquired_lot_amount: RP2Decimal = ZERO From 201cc48cb9b0f98893f85bf5ebba5a448d7b0b95 Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Thu, 30 May 2024 10:56:28 -0700 Subject: [PATCH 5/6] Add assert on lot_candidates --- src/rp2/accounting_engine.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rp2/accounting_engine.py b/src/rp2/accounting_engine.py index 3e3357c..265122f 100644 --- a/src/rp2/accounting_engine.py +++ b/src/rp2/accounting_engine.py @@ -207,18 +207,18 @@ def get_acquired_lot_for_taxable_event( raise RP2RuntimeError("Internal error: acquired_lot incongruence in accounting logic") method = self._get_accounting_method(taxable_event.timestamp.year) lot_candidates: Optional[AcquiredLotCandidates] = self.__years_2_lot_candidates.find_max_value_less_than(taxable_event.timestamp.year) - - if lot_candidates: - lot_candidates.set_to_index(acquired_lot_and_index.index) - acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( - lot_candidates, taxable_event, new_taxable_event_amount + # lot_candidates is 1:1 with acquired_lot_and_index + assert lot_candidates is not None + lot_candidates.set_to_index(acquired_lot_and_index.index) + acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( + lot_candidates, taxable_event, new_taxable_event_amount + ) + if acquired_lot_and_amount: + return TaxableEventAndAcquiredLot( + taxable_event=taxable_event, + acquired_lot=acquired_lot_and_amount.acquired_lot, + taxable_event_amount=new_taxable_event_amount, + acquired_lot_amount=acquired_lot_and_amount.amount, ) - if acquired_lot_and_amount: - return TaxableEventAndAcquiredLot( - taxable_event=taxable_event, - acquired_lot=acquired_lot_and_amount.acquired_lot, - taxable_event_amount=new_taxable_event_amount, - acquired_lot_amount=acquired_lot_and_amount.amount, - ) raise AcquiredLotsExhaustedException() From d7031e500d2a6e115f329e5c56382e1584989ccf Mon Sep 17 00:00:00 2001 From: Christopher Whelan Date: Thu, 30 May 2024 21:11:22 -0700 Subject: [PATCH 6/6] Make bandit happy with if statement --- src/rp2/accounting_engine.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rp2/accounting_engine.py b/src/rp2/accounting_engine.py index 265122f..9ff3136 100644 --- a/src/rp2/accounting_engine.py +++ b/src/rp2/accounting_engine.py @@ -207,18 +207,18 @@ def get_acquired_lot_for_taxable_event( raise RP2RuntimeError("Internal error: acquired_lot incongruence in accounting logic") method = self._get_accounting_method(taxable_event.timestamp.year) lot_candidates: Optional[AcquiredLotCandidates] = self.__years_2_lot_candidates.find_max_value_less_than(taxable_event.timestamp.year) - # lot_candidates is 1:1 with acquired_lot_and_index - assert lot_candidates is not None - lot_candidates.set_to_index(acquired_lot_and_index.index) - acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( - lot_candidates, taxable_event, new_taxable_event_amount - ) - if acquired_lot_and_amount: - return TaxableEventAndAcquiredLot( - taxable_event=taxable_event, - acquired_lot=acquired_lot_and_amount.acquired_lot, - taxable_event_amount=new_taxable_event_amount, - acquired_lot_amount=acquired_lot_and_amount.amount, + # lot_candidates is 1:1 with acquired_lot_and_index, should always be True + if lot_candidates: + lot_candidates.set_to_index(acquired_lot_and_index.index) + acquired_lot_and_amount: Optional[AcquiredLotAndAmount] = method.seek_non_exhausted_acquired_lot( + lot_candidates, taxable_event, new_taxable_event_amount ) + if acquired_lot_and_amount: + return TaxableEventAndAcquiredLot( + taxable_event=taxable_event, + acquired_lot=acquired_lot_and_amount.acquired_lot, + taxable_event_amount=new_taxable_event_amount, + acquired_lot_amount=acquired_lot_and_amount.amount, + ) raise AcquiredLotsExhaustedException()