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

Split devenv into different node profiles. #307

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

CAGS295
Copy link
Contributor

@CAGS295 CAGS295 commented Oct 19, 2023

Summary of Changes

There's been some debate about the scope of devenvs services. I'm exploring how to enable multiple 'devenv' profiles to cover a wider range of applications (core dev, third-party dev, testing). The immediate need is a minimal node with a local Romeo instance. I added a minimal profile with (bitcoind, stacks + RPC, electrs) for local romeo development.

  • Split the docker-compose file. A new file contains fewer services meant for sbtc developers that spin Romeo locally.
  • Refactor up/down commands to support upping and downing 'min'/'full' nodes.
  • Sets precedence for a dedicated 'node profile' for integration tests.

Testing

Risks

Didn't fully test all util scripts. I don't expect any big complications, though.

How were these changes tested?

manual runs.

./up.sh 
 ./up.sh min --help
 ./up.sh min min
 ./up.sh min full

 ./up.sh full
 ./down.sh full

 ./up.sh min
 ./down.sh min

 ./up.sh full
 ./up.sh min
 ./down.sh full
 ./up.sh min
 ./down.sh full
 ./up.sh full

 ./up.sh full
 ./utils/mine_btc.sh 10

What future testing should occur?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #307 (6c07ff7) into main (c1b5abd) will increase coverage by 1.62%.
Report is 8 commits behind head on main.
The diff coverage is n/a.

❗ Current head 6c07ff7 differs from pull request most recent head 89516e2. Consider uploading reports for the commit 89516e2 to get more accurate results

@@            Coverage Diff             @@
##             main     #307      +/-   ##
==========================================
+ Coverage   41.49%   43.12%   +1.62%     
==========================================
  Files          40       45       +5     
  Lines        5111     5359     +248     
  Branches        0       47      +47     
==========================================
+ Hits         2121     2311     +190     
- Misses       2990     3047      +57     
- Partials        0        1       +1     
Flag Coverage Δ
unittests 76.61% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 5 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@CAGS295 CAGS295 added enhancement New feature or request alpha-romeo devenv Development Environment labels Oct 19, 2023
@CAGS295 CAGS295 marked this pull request as ready for review October 20, 2023 17:24
@CAGS295
Copy link
Contributor Author

CAGS295 commented Oct 20, 2023

Partially addresses #282. We would still need to revisit utils in another PR.

Copy link
Collaborator

@friedger friedger left a comment

Choose a reason for hiding this comment

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

Looks good, haven't tested it yet.

@CAGS295 CAGS295 mentioned this pull request Oct 25, 2023
8 tasks
@friedger
Copy link
Collaborator

The goal should probably be that we can run several signers as defined here: https://github.com/stacks-network/stacks-blockchain/blob/next/stacks-signer/README.md#generate-files

@CAGS295
Copy link
Contributor Author

CAGS295 commented Oct 27, 2023

The goal should probably be that we can run several signers as defined here: https://github.com/stacks-network/stacks-blockchain/blob/next/stacks-signer/README.md#generate-files

Thanks for the heads up. My immediate goal is to unblock the integration pipeline. Adding support for a 'signer ring' profile sounds good too. Once we have more details, I can help with that if needed.

I can see so far a few different use cases.

  1. full-node for third party users exploring sBTC.
  2. min for dev with a local romeo server.
  3. integration running services required for testing only.
  4. signer-ring for signing rounds.

@CAGS295 CAGS295 self-assigned this Nov 9, 2023
@CLAassistant
Copy link

CLAassistant commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha-romeo devenv Development Environment enhancement New feature or request
Projects
No open projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

3 participants