[ADP-3443] Add roll forward and backward of TxHistory to deposit wallet #4828
+1,366
−182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paolino
changed the base branch from
master
to
paolino/ADP-3443/use-fingertrees-for-time-operations-on-tx-history
paolino
force-pushed
the
paolino/ADP-3443/use-fingertrees-for-time-operations-on-tx-history
branch
from
70a7c5b
to
2dc09f2
Compare
paolino
force-pushed
the
paolino/ADP-3443/add-rollforward-to-txhistory
branch
from
73ace9a
to
3eea066
Compare
Base automatically changed from
paolino/ADP-3443/use-fingertrees-for-time-operations-on-tx-history
to
master
November 1, 2024 13:29
paolino
force-pushed
the
paolino/ADP-3443/add-rollforward-to-txhistory
branch
from
2d59d5d
to
77143ef
Compare
paolino
changed the title
paolino
changed the title
There was a problem hiding this comment.
First look. 🤓
This looks good, but there are two correctness issues and one request on naming.
- Could you rename
ResolveSlottoLookupTimeFromSlot? I think that the latter name is more descriptive. - Could you move the computation of the
ValueTransfersfor the transactions toCardano.Wallet.Deposit.Pure.Balance? The expression you wrote is not quite correct, and we need to be careful here — hence isolate it in a separate module, and just pass the data around in the other modules. - The rollback of the
TxHistoryneeds to include the block to which it is rolled back. Could you use the namedropAfterinstead ofbefore?
lib/customer-deposit-wallet/test/unit/Cardano/Wallet/Deposit/PureSpec.hs
Outdated
Show resolved
Hide resolved
lib/customer-deposit-wallet/src/Cardano/Wallet/Deposit/Pure/API/TxHistory.hs
Outdated
Show resolved
Hide resolved
paolino
force-pushed
the
paolino/ADP-3443/add-rollforward-to-txhistory
branch
9 times, most recently
from
966c513
to
41cb6d2
Compare
paolino
force-pushed
the
paolino/ADP-3443/add-rollforward-to-txhistory
branch
from
41cb6d2
to
8bfb18d
Compare
The instance is meant to not repeat times when during concatenation
paolino
force-pushed
the
paolino/ADP-3443/add-rollforward-to-txhistory
branch
from
8bfb18d
to
9fe409a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ADP-3443