Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Bloomchain #1014

Merged
merged 24 commits into from May 26, 2016
Merged

Bloomchain #1014

merged 24 commits into from May 26, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Apr 28, 2016

requires #996 and #997

small fixes, optimizations and removing duplicated logic

  • blockchain::BlockChainConfig moved to its own file and renamed to just Config
  • removed old chainfilter module and everything related to it, eg. blockchain::BloomIndexer. Use bloomchain crate instead.
  • in extras database type byte is the first byte, instead being the last. That should improve query performance
  • BlocksBlooms database keys are smaller - 6 bytes (33 bytes before).
  • BlockHashes database keys are smaller - 5 bytes( 33 bytes before).
  • change client database version to 6.0

@debris debris added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Apr 28, 2016
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels May 4, 2016
@@ -113,6 +113,7 @@ fn setup_rpc_server(apis: Vec<&str>, deps: &Arc<Dependencies>) -> Server {
server.add_delegate(EthcoreClient::new(&deps.miner, deps.logger.clone(), deps.settings.clone()).to_delegate())
},
"traces" => {
// not adding to modules, since `traces` is not supported in geth
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right! incorrect merge 👍

@gavofyork
Copy link
Contributor

might be worth staging somewhere other than master or we'll be resyncing multiple times with the current DB changes...

@debris
Copy link
Collaborator Author

debris commented May 4, 2016

I can keep this pr up to date until there is a right moment to merge it

@gavofyork gavofyork added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A0-pleasereview 🤓 Pull request needs code review. labels May 5, 2016
@NikVolf
Copy link
Contributor

NikVolf commented May 7, 2016

btw can't the database be auto-upgraded somehow?

@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels May 25, 2016
@debris
Copy link
Collaborator Author

debris commented May 25, 2016

Added migrations

let mut result = H264::default();
result[0] = i as u8;
unsafe {
ptr::copy(hash.as_ptr(), result.as_mut_ptr().offset(1), 32);
Copy link
Collaborator

Choose a reason for hiding this comment

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

prefer clone_from_slice to unsafe code.

let result_key = &mut result[1 .. ];
result_key.clone_from_slice(hash);

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels May 25, 2016
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels May 25, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels May 25, 2016
}

struct CacheManager {
cache_usage: VecDeque<HashSet<CacheID>>,
in_use: HashSet<CacheID>,
}

impl bc::group::BloomGroupDatabase for BlockChain {
fn blooms_at(&self, position: &bc::group::GroupPosition) -> Option<bc::group::BloomGroup> {

Copy link
Contributor

Choose a reason for hiding this comment

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

superfluous line

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels May 26, 2016
@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels May 26, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels May 26, 2016
@gavofyork gavofyork merged commit 7370776 into master May 26, 2016
@gavofyork gavofyork deleted the bloomchain branch May 26, 2016 16:24
@ethshan ethshan mentioned this pull request Sep 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants