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

Add hash of dao state #2532

Merged
merged 33 commits into from Mar 16, 2019

Conversation

Projects
None yet
2 participants
@ManfredKarrer
Copy link
Member

commented Mar 11, 2019

@sqrrm
Here is the current state:
Screen Shot 2019-03-11 at 18 41 13

It is still missing the p2p network data monitoring and tests with reorgs.

ManfredKarrer added some commits Mar 8, 2019

Remove optional setting of pubKeyScript
We want to have a deterministic dao state. The PubKeyScript in the
TxOutput was the only optional field which was only set in the
dumpBlockchainData program argument was set as it was only required for
the json dump for the explorer. It has 50 bytes of data which is about
20% of the txOutput data size. We prefer atm to avoid additional
complexity which would be created if we would handle that optional
field (e.g. exclude from hash creation).
The property might be useful as well for other use cases in future.
Create chain of dao state hashes for monitoring consensus issues
We create a chain of hashes of the dao state starting from the genesis
block height and using the previous hash in the hash. This ensures that
the history need to be correct if a particular hash at a block height is
correct. We request from seed nodes the last 10 hashes and broadcast to
our peers our hash at each new block. We build our list in memory and
listen on the new onSnapShotApplied event to start building our chain
from the genesis height up to the last snapshot block and after that
from each parsed block.
If we detect a mismatch we store it in a collection and the UI can show
a warning to the user.
We added also the onDaoStateChanged handler to the DaoStateListener.
This event is called after all parsing is completed and listeners have
completed their work. We must not use time based delays in the listener
code otherwise we would get changed our dao state after that event.
To detect such we added a assert method to throw an exception if the
dao state gets changed after the allowDaoStateChange is set to false.
Add parseBlockchainComplete check
Add parseBlockchainComplete check for onParseBlockChainComplete
before calling onParseBlockChainComplete to avoid duplicated calls of
onParseBlockChainComplete. Happened if there was only one seed node.
Add UI for Dao state monitor
Refactor dao state monitor domain.
Add support for requesting all hashed from genesis from a peer which is
in conflict (can help to find at which block the issue started).

@ManfredKarrer ManfredKarrer added this to the v0.9.6 milestone Mar 11, 2019

@ManfredKarrer ManfredKarrer requested a review from sqrrm Mar 11, 2019

@ManfredKarrer ManfredKarrer self-assigned this Mar 11, 2019

ManfredKarrer added some commits Mar 12, 2019

@ManfredKarrer

This comment has been minimized.

Copy link
Member Author

commented Mar 12, 2019

Tested with DAO TESTNET and looks good so far:

Screen Shot 2019-03-12 at 00 33 51

ManfredKarrer added some commits Mar 13, 2019

Add snapshot support for hashChain
- Add hashChain to snapshot
- Use TreeMap instead of Map to have deterministic order
- Use eventCoordinator to make correct order of execution transparent
- Fix missing applying of nonBsqTxOutputMap at applySnapshot
@@ -45,9 +45,10 @@
protected final long value;
protected final String txId;

// Only set if dumpBlockchainData is true
// Before v0.9.6 it was only set if dumpBlockchainData was set to true but we changed that with 0.9.6
// so that is is always set. We still need to support it because of backward compatibility.

This comment has been minimized.

Copy link
@sqrrm

sqrrm Mar 14, 2019

Member

Perhaps we should do a cleanup before starting testing on mainnet. That would break the regtestnet we have running now though.

Show resolved Hide resolved core/src/main/java/bisq/core/dao/state/DaoStateService.java Outdated
Show resolved Hide resolved .../main/java/bisq/core/dao/state/monitoring/DaoStateMonitoringService.java Outdated
/**
* Monitors the DaoState with using a hash fo the complete daoState and make it accessible to the network for
* so we can detect quickly if any consensus issue arise. The data does not contain any private user
* data so sharing it on demand has no privacy concerns.

This comment has been minimized.

Copy link
@sqrrm

sqrrm Mar 14, 2019

Member

Could this be an attack vector, allowing too many requests as a DOS?

This comment has been minimized.

Copy link
@ManfredKarrer

ManfredKarrer Mar 15, 2019

Author Member

On the P2P network level it would be disconnected it it gets heavy, but basically yes. We should consider to limit it somehow.

Show resolved Hide resolved .../main/java/bisq/core/dao/state/monitoring/DaoStateMonitoringService.java Outdated
Show resolved Hide resolved .../main/java/bisq/core/dao/state/monitoring/DaoStateMonitoringService.java Outdated
Show resolved Hide resolved .../main/java/bisq/core/dao/state/monitoring/DaoStateMonitoringService.java Outdated

ManfredKarrer added some commits Mar 15, 2019

Refactor dao state monitor
- Move dao state monitor classes to other package structure
- Add support for proposal payload monitor (WIP)
- Use generics and sub classes for common functionality
Add num proposals
- Cleanups
- Improve logs

ManfredKarrer added some commits Mar 16, 2019

Fix table layout issue
The setFitToRowsForTableView call at activate fixes the issue that the
views sometimes do not render when changing views.

@ManfredKarrer ManfredKarrer marked this pull request as ready for review Mar 16, 2019

@ManfredKarrer ManfredKarrer requested a review from ripcurlx as a code owner Mar 16, 2019

@ManfredKarrer

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2019

@sqrrm I think it is now complete. Could you cross check again? I tested re-orgs as well and seems that all works so far. Tested also backward compatibility with master and all ok.

ManfredKarrer added some commits Mar 16, 2019

@sqrrm
Copy link
Member

left a comment

My comment on throwing an exception only happened midway through these commits. I still think it can happen but couldn't recreate it.

Everything looks good while running on the current tip. I think it's basically good to go.

ManfredKarrer added some commits Mar 16, 2019

Change file name for DaoStateStore db file to DaoStateStore2, Cleanup
- Change onDaoStateBlockChainChanged to onChangeAfterBatchProcessing
to make it more clear that it is only called after parsing of
blockchain is completed.
- Change file name for DaoStateStore db file to DaoStateStore2
- Remove TODOs and outdated comments
- Increase delay for broadcasting

@ManfredKarrer ManfredKarrer merged commit 545eb8c into bisq-network:master Mar 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ManfredKarrer ManfredKarrer deleted the ManfredKarrer:add-hash-of-dao-state branch Mar 16, 2019

@sqrrm sqrrm referenced this pull request Mar 30, 2019

Closed

For March 2019 #253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.