Yet another fix for reported withdrawals in transactions + underflow in case of key deposit reclaim #2010
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
We haven't been quite careful here when introducing the reward redemption and the transaction amount are looking weird again.
This commit fixes several issues:
1. It only counts withdrawals on the "spent" side of the balance if they are coming from OUR reward account. Indeed, in the case of external withdrawals, the money is coming from elsewhere and not from the wallet itself.
2. Fix an underflow in the amount calculation in the case where we spent less than we receive. This can be the case when:
a. We are redeeming from an external account and the reward brings more than the fee. From the redeeming wallet, it'll look like the wallet is receiving money.
b. We are reclaiming a key deposit back, and it brings more money than what's actually spent.
3. Discover transactions that are spending our withdrawals without belonging to our wallet. This happens when the reward is redeemed from another wallet. That transaction should still show up in the redeemed wallet, without which rewards would just "vanish" without any trace reported by the wallet.
paweljakubas
approved these changes
Aug 8, 2020
There was a problem hiding this comment.
I thought we have already ourWithdrawal in model ... and what is left to be accounted are inputs when multisig txs are added. Using distance seems fine to avoid underflow error. So I bet in this or another PR the ourWithdrawal logic is going to be only in Model, right?
|
bors r+ |
KtorZ
added
the
RESOLVING ISSUE
Build succeeded |
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.
Issue Number
#2009 + TODO
Overview
5757a55
📍 highlight reported failure in integration scenarios by adding more assertions
d5b4335
📍 report our withdrawals differently from external withdrawals.
We haven't been quite careful here when introducing the reward redemption and the transaction amount are looking weird again.
This commit fixes several issues:
It only counts withdrawals on the "spent" side of the balance if they are coming from OUR reward account. Indeed, in the case of external withdrawals, the money is coming from elsewhere and not from the wallet itself.
Fix an underflow in the amount calculation in the case where we spent less than we receive. This can be the case when:
a. We are redeeming from an external account and the reward brings more than the fee. From the redeeming wallet, it'll look like the wallet is receiving money.
b. We are reclaiming a key deposit back, and it brings more money than what's actually spent.
Discover transactions that are spending our withdrawals without belonging to our wallet. This happens when the reward is redeemed from another wallet. That transaction should still show up in the redeemed wallet, without which rewards would just "vanish" without any trace reported by the wallet.
Comments