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

Feat/stackerdb rpc #3848

Merged
merged 74 commits into from
Sep 2, 2023
Merged

Feat/stackerdb rpc #3848

merged 74 commits into from
Sep 2, 2023

Conversation

jcnelson
Copy link
Member

This PR implements the RPC interface for StackerDB.

This is a WIP for the next few days as more product requirements and stakeholder feedback are gathered and addressed. Also, the API documentation needs to be written.

…ks, and listing chunk metadata in a stackerdb
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #3848 (ef6c5a4) into develop (81129be) will decrease coverage by 0.01%.
Report is 1 commits behind head on develop.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3848      +/-   ##
===========================================
- Coverage     0.16%    0.16%   -0.01%     
===========================================
  Files          319      322       +3     
  Lines       285503   287639    +2136     
===========================================
  Hits           469      469              
- Misses      285034   287170    +2136     
Files Changed Coverage Δ
libstackerdb/src/libstackerdb.rs 0.00% <0.00%> (ø)
libstackerdb/src/tests/mod.rs 0.00% <0.00%> (ø)
stackslib/src/chainstate/stacks/db/blocks.rs 0.00% <0.00%> (ø)
stackslib/src/chainstate/stacks/db/mod.rs 0.00% <ø> (ø)
stackslib/src/core/mempool.rs 0.00% <ø> (ø)
stackslib/src/core/tests/mod.rs 0.00% <ø> (ø)
stackslib/src/lib.rs 0.00% <ø> (ø)
stackslib/src/main.rs 0.00% <0.00%> (ø)
stackslib/src/net/asn.rs 0.00% <ø> (ø)
stackslib/src/net/chat.rs 0.00% <0.00%> (ø)
... and 33 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jcnelson
Copy link
Member Author

jcnelson commented Aug 23, 2023

Hi reviewers,

I'm not sure why yet, but the "Full Genesis Test" only fails due to bitcoind_integration_test failing. However, this passes locally for me. Moreover, I've updated the Bitcoin Docker file to pull in Bitcoin Core v25, which is what I have installed locally. Not sure what to do here, since GH actions doesn't allow SSH access to the container.

EDIT: the core issue is that the miner does not find UTXOs, which is weird. I'll try to reproduce locally.

EDIT 2: I was unable to reproduce locally even when using the very same bitcoind that the new Docker image stipulates, but I admittedly did not try it out in Docker. Will do that next.

@jcnelson
Copy link
Member Author

@kantai @obycode @jferrant This is ready for re-review now that all tests are passing.

Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM

#[cfg(test)]
mod tests;

#[derive(Debug)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

A lot of this could be simplified if we use thiserror. Not sure if there is a reason we currently don't use it throughout the code base though.

@@ -1074,6 +1077,35 @@ simulating a miner.
process::exit(0);
}

if argv[1] == "post-stackerdb" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than manually parsing args, can we use something like clap?

Copy link
Collaborator

@jferrant jferrant left a comment

Choose a reason for hiding this comment

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

Just a couple comments about cleanup but are not blockers by any means.

@jcnelson jcnelson changed the base branch from feat/stackerdb-config to develop September 1, 2023 22:49
@jcnelson jcnelson merged commit f480846 into develop Sep 2, 2023
2 checks passed
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.

4 participants