Skip to content

Conversation

@monyedavid
Copy link
Collaborator

Unconfirmed balance wrong when a payment is made
Resolves issues #177 #141

  - BuildUnconfirmedBalance utility
- BuildUnconfirmedBalanceSpec
- MempoolSpec
   - get confirmed balance (tokens) of address
   - get confirmed Balance (nanoErgs) of address
   - get confirmed balance (tokens) of address
   - get confirmed Balance (nanoErgs) of address
Copy link
Member

@oskin1 oskin1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Duplicated changes.

@monyedavid monyedavid changed the base branch from master to base_pr May 22, 2022 22:37
@monyedavid monyedavid requested a review from oskin1 May 22, 2022 22:43
@monyedavid
Copy link
Collaborator Author

Same here. Duplicated changes.

@oskin1 retargeted PR to base_pr

@monyedavid monyedavid mentioned this pull request May 22, 2022
# Conflicts:
#	modules/explorer-api/src/main/scala/org/ergoplatform/explorer/http/api/v1/services/Mempool.scala
#	modules/explorer-api/src/test/scala/org/ergoplatform/explorer/v1/services/AddressesSpec.scala
#	modules/explorer-api/src/test/scala/org/ergoplatform/explorer/v1/services/MempoolSpec.scala
Comment on lines 69 to 73
def totalBalanceOf_(address: Address): F[TotalBalance] =
for {
confirmed <- confirmedBalanceOf(address, 0)
unconfirmed <- memprops.getUnconfirmedBalanceByAddress(address, confirmed)
} yield TotalBalance(confirmed, unconfirmed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this method does the same thing as totalBalanceOf but in a less efficient way. memprops.getUnconfirmedBalanceByAddress operates on the same data from database as

        offChainBalance <- uOutputRepo.sumUnspentByErgoTree(tree)
        offChainAssets  <- uAssetRepo.aggregateUnspentByErgoTree(tree)

The difference only is that old code aggregates balances inside DB and you does all this in the application's runtime.
I believe this doesn't solve the problem.

Copy link
Collaborator Author

@monyedavid monyedavid May 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oskin1 this totalBalanceOf_ uses memprops.getUnconfirmedBalanceByAddress
which generates unconfirmed balance with reference to:

  • (of new unconfirmed outputs in mempool)
  • (confirmed outputs spent in mempool)
  • and already calculated confirmed balance from confirmedBalanceOf(address, 0)

the old is not inclusive of mempool transactions
so memprops.getUnconfirmedBalanceByAddress(address, confirmed) using all that data; if you have 10Ergs, send 2Erg and received 3Erg your unconfirmed balance should be 11Erg
the old one doesn't return this expected value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh,got it. The old one includes outputs from unconfirmed transactions, so all you need to do is to sum unconfirmed inputs and subtract them from total unconfirmed balance.

Copy link
Collaborator Author

@monyedavid monyedavid May 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats what I did the first time, it didn't work in cases when an address sends out & receives tokens during the same "confirmation period"

the algorithm to correctly get the new unconfirmed balance works something along the lines of:
foldLeft transactions(items => item(Tx)):
- determine if transaction is a debit/credit via inputErgoTree(source of funds) ☑️
- determine useable outputs:
- output.ergoTree != wallet.ergoTree || for debit transactions
- output.ergoTree == wallet.ergoTree || for credit transactions

defined in this utility:
BuildUnconfirmedBalance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate why simple aggregation of unconfirmed inputs/outputs doesn't work?
You get the same set of inputs/outputs inside unconfirmed transactions, so the result should be the same, right?

@monyedavid monyedavid requested a review from oskin1 June 7, 2022 06:25
@monyedavid monyedavid marked this pull request as draft June 9, 2022 11:49
Base automatically changed from base_pr to master June 9, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants