[bisq-network/bisq] Add transient tx map to DaoState to speed up getTx queries (#3773)

chimp1984 notifications at github.com
Wed Dec 11 17:22:36 UTC 2019


@julianknutsen 

> The reality is that this code will need to change if we want performance gains. I think it would be a good idea to get everyone on the same page with what can/can't be done in this area and how much testing needs to be a prerequiste to changes.

Thanks for bringing up the broader discussion!

My view is that the DAO code should be only touched if there is a very clear reason. Bugs justify the risk to change the code and critical performance issues. There are some layer in the DAO code which is less critical like requesting the BSQ blocks. But the core, the parsing, DaoState and data structures are highly critical consensus code and have to been taken with lot of care.

Regarding testing:
Of course making the code more testable and adding tests would be great. If that will require non purely technical refactorings (low risk changes) I think we have to discuss the added value and the risk as well to check our overall priorities.

For the current state testing can be done by creating all types of DAO transactions and use all DAO features (burn BSQ, voting,...) and create a few cycles with param changes (also radical changes - there we found and fixed a bug which only occurred at more radical changes). The DaoState must be then the same from the original version and the changed version. To resync from genesis is required and running as full DAO node is recommended as well. I usually run 1 full node and one lite node to cover both paths. For dev testing regtest is enough but for the final test this has to be done with the mainnet. A resync with the lite node causes a bit of stress for the live seed nodes, so it should be done only if needed. They only deliver max. 6000 blocks, so it will require repeated BSQ block requests. Also it takes quite a bit of time, but comparing the hashes of the last block gives a the best assurance we have atm regarding the DAO. 

By the way: As in the past we had problems with merged PRs to the dao domain which have been problematic, we decided that the code inside the dao package in the core module requires an explicit ACK from @ManfredKarrer (see https://github.com/bisq-network/proposals/issues/119). 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3773#issuecomment-564647136
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191211/84b7fc5a/attachment-0001.html>


More information about the bisq-github mailing list