Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new burningman accounting file recreated from scratch. #6740

Merged

Conversation

HenrikJannsen
Copy link
Collaborator

Use new burningman accounting file recreated from scratch.
Remove old file.

The current accounting data are missing about 10% of transactions due to threading issues at processing the transactions in earlier versions.

Remove old file.

The current accounting data are missing about 10% of transactions due to threading issues at processing the transactions in earlier versions.
@HenrikJannsen HenrikJannsen force-pushed the rename_bm_accounting_store branch from e2b6d4a to 16bbef9 Compare June 23, 2023 20:20
Signed-off-by: HenrikJannsen <boilingfrog@gmx.com>
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

ACK
The numbers are close to the values from my full node.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The BM BTC received amount shown is slightly higher than the real amount received. Any ideas why?

@HenrikJannsen
Copy link
Collaborator Author

The BM BTC received amount shown is slightly higher than the real amount received. Any ideas why?

Have you checked if there was payments to the Bisq internal addresses (either BTC or BSQ wallet).
Another reason can be that false positives interpret a transaction wrongly if there have been other transaction related to the address. The DPT has less risk to get false positives but the fee tx has as its has less metadata we can check, only amount range and some other properties.

@ghost
Copy link

ghost commented Jun 28, 2023

Have you checked if there was payments to the Bisq internal addresses (either BTC or BSQ wallet).

It seems to be this. A BMs receiver address is sometimes chosen from a seemingly random DAO comp cycle. In my case from cycle 36 and 41. Only 2 transactions out of several thousand, all the rest went to the specified custom address. I checked some other BM and they exhibit the same quirk.

Not related to this particular PR though.

@HenrikJannsen
Copy link
Collaborator Author

Have you checked if there was payments to the Bisq internal addresses (either BTC or BSQ wallet).

It seems to be this. A BMs receiver address is sometimes chosen from a seemingly random DAO comp cycle. In my case from cycle 36 and 41. Only 2 transactions out of several thousand, all the rest went to the specified custom address. I checked some other BM and they exhibit the same quirk.

Not related to this particular PR though.

I guess that can happen for trade fees if users do not have the correct DAO data (comp request with the external address).

@alejandrogarcia83 alejandrogarcia83 added this to the v1.9.11 milestone Jun 28, 2023
Copy link
Contributor

@alejandrogarcia83 alejandrogarcia83 left a comment

Choose a reason for hiding this comment

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

utACK

@alejandrogarcia83 alejandrogarcia83 merged commit 8c70dd6 into bisq-network:master Jun 28, 2023
alejandrogarcia83 added a commit to alejandrogarcia83/bisq that referenced this pull request Jun 30, 2023
PR bisq-network#6740 was a diff against master (instead against the release/v1.9.11
branch). The previous merge into the release branch corrupted the
acounting file.
@HenrikJannsen HenrikJannsen deleted the rename_bm_accounting_store branch July 1, 2023 12:27
helixx87 pushed a commit to helixx87/bisq that referenced this pull request Feb 13, 2024
PR bisq-network#6740 was a diff against master (instead against the release/v1.9.11
branch). The previous merge into the release branch corrupted the
acounting file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants