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

Fix premature validation call #5894

Conversation

chimp1984
Copy link
Contributor

When syncing from genesis the number of blocks are limited so we get the
onParseBlockCompleteAfterBatchProcessing called each time when the received
blocks are processed, and as we are not at wallet height we repeat requesting
blocks. But the new check for the BTC recipient triggers a resync from resource call.
We add now a check that we do this check only once the wallet is synced and our
block height from dao state matches wallet blockheight.

Fixes #5882

When syncing from genesis the number of blocks are limited so we get the
`onParseBlockCompleteAfterBatchProcessing` called each time when the received
 blocks are processed, and as we are not at wallet height we repeat requesting
 blocks. But the new check for the BTC recipient triggers a resync from resource call.
We add now a check that we do this check only once the wallet is synced and our
block height from dao state matches wallet blockheight.
@ghost
Copy link

ghost commented Dec 4, 2021

Getting a lot of repeated errors in the log, e.g.

Dec-03 22:40:11.073 [JavaFX Application Thread] WARN  b.c.d.g.p.ProposalValidator: proposal data fields are invalid.     proposal=ReimbursementProposal{     requestedBsq=2590441,     bsqAddress='Bbc1qdl8830cvr2uznwxc9qdmqq9nfqvv78d3sknjtk'
} Proposal{     txId='e037770c6ff4439c7d7276bbd9b7c744a52e130c9ea8dd64a7e1fe27ceaa6b10',
 name='refund-agent2',     link='https://bisq.network/dao-reimbursement/1013',
 txId='e037770c6ff4439c7d7276bbd9b7c744a52e130c9ea8dd64a7e1fe27ceaa6b10',     version=1,     creationDate=Sat Oct 23 14:44:17 CDT 2021
}, error=ProposalValidationException{     requestedBsq=null,     minRequestAmount=null,     tx=null
bisq.core.dao.governance.proposal.ProposalValidationException: java.lang.IllegalArgumentException: Requested BSQ must not exceed 10000 BSQ 
Dec-03 22:40:11.092 [JavaFX Application Thread] WARN  b.c.d.m.DaoStateMonitoringService: New block must be 1 block above previous block. height=627833, daoStateBlockChain.getLast().getHeight()=712495 
Dec-03 22:40:11.092 [JavaFX Application Thread] INFO  b.c.d.node.parser.BlockParser: Parsing 0 transactions at block height 627833 took 38 ms 
Dec-03 22:40:11.093 [JavaFX Application Thread] INFO  bisq.core.dao.node.BsqNode: We have already a block with the height of the new block. Height of new block=627782 
Dec-03 22:40:11.093 [JavaFX Application Thread] INFO  bisq.core.dao.node.BsqNode: We have already a block with the height of the new block. Height of new block=627551 

Left it syncing for an hour without finishing.

@chimp1984
Copy link
Contributor Author

chimp1984 commented Dec 4, 2021

Resync from genesis? That will likely take quite some time as the number of blocks is limited, so it repeats to request blocks.

@chimp1984
Copy link
Contributor Author

Doing a resync from genesis as well now. Each blocks request is limited to 6000 blocks. The parsing is fast now, about 10 sec or so, but the blocks requests are slow. So far I dont see errors...

@chimp1984
Copy link
Contributor Author

Got my lite node (and lite monitor mode) synced from genesis and all looks ok. Also tried with 2 other lite nodes and restarted several times and all was ok. But once I got a conflict with seed nodes with request to resync, but I just restarted and it was resolved after restart. So it seems there is some bug still with some edge cases but could not find it or find how to reproduce it. Also on regtest I tried to reproduce but failed.

When doing a resync from genesis the number of blocks is limited to 6000
so that requires lots of requests and with that increases risk of broken
connections. Giving more tolerance for retries avoids that the user has
to restart the app.
@ghost
Copy link

ghost commented Dec 4, 2021

Got the sync completed ok now, unsure what was the problem before. I'll keep trying it.

@ghost
Copy link

ghost commented Dec 5, 2021

I've been doing syncs today without problems. My earlier error must have been a build glitch.

Observations:

  • For a new user, fresh data directory, all default settings (lite node). DAO sync is taking an hour or more.
  • Sometimes it stops syncing without any indication. I have to restart Bisq to get it to continue.

@chimp1984
Copy link
Contributor Author

* For a new user, fresh data directory, all default settings (lite node).  DAO sync is taking an hour or more.

Sync from genesis? Or from resources (default). From resources should be very fast (10 sec or so).

@ghost
Copy link

ghost commented Dec 5, 2021

New user, fresh data directory. This message is reported at startup:

java.lang.IllegalArgumentException: URI is not hierarchical
at java.base/java.io.File.<init>(File.java:418)
at bisq.core.dao.state.storage.BsqBlocksStorageService.copyFromResources(BsqBlocksStorageService.java:124)
at bisq.core.dao.state.storage.DaoStateStorageService.lambda$readFromResources$4(DaoStateStorageService.java:119)
at java.base/java.lang.Thread.run(Thread.java:834)

@chimp1984
Copy link
Contributor Author

I cannot reproduce that on OSX running from IDEA.
Did you run is from a jar or binary? Which OS?
The .toURI() call is used several times in Bisq.

Can you check whats the value of dirUrl?
Should be:
file:/Users/[USER]/Java/bisq/bisq/p2p/out/production/resources/BsqBlocks_BTC_MAINNET
Maybe the user path has some chars which causes the issue?

@ghost
Copy link

ghost commented Dec 5, 2021

  • Linux / Ubuntu.
  • releases/v1.8.0 branch + PR 5894 patch
  • build done via ./gradlew clean build
  • executed via ./bisq-desktop

this is the value of dirUrl:
jar:file:/home/username/Documents/dev/bisq/lib/p2p.jar!/BsqBlocks_BTC_MAINNET

I made a PR to fix the issue. #5898

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2021

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2021

Tested those commits on top of v1.8.0 with an outdated Mainnet client and now everything worked as expected. 👍

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

ACK - Tested it no Mainnet

@ripcurlx ripcurlx added this to the v1.8.0 milestone Dec 6, 2021
@ripcurlx ripcurlx merged commit f5db85b into bisq-network:master Dec 6, 2021
@chimp1984 chimp1984 deleted the check-recipient-address-only-after-wallet-is-synced branch December 7, 2021 15:07
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.

[1.8.0] New user DAO sync required 4 restarts to complete
2 participants