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 signer and update versions #128

Merged

Conversation

bowtiedbot
Copy link
Contributor

@bowtiedbot bowtiedbot commented Oct 25, 2024

Added signer docker compose file as extra service, which can be started with an optional flag on all available networks.

Also updated node and API versions to the latest ones:

  • stacks-node: 3.0.0.0.0
  • stacks-blockchain-api: 8.1.2
  • stacks-signer: 3.0.0.0.0.0

Added signer docker compose file as extra service, which can be started with an optional flag on all available networks. Updated node and API versions to the latest ones.
@CLAassistant
Copy link

CLAassistant commented Oct 25, 2024

CLA assistant check
All committers have signed the CLA.

@wileyj
Copy link
Collaborator

wileyj commented Oct 25, 2024

unless i'm misreading this, you're proposing to start a signer as flag i.e. -f signer similar to how nginx proxy is started.
however, i don't see a compose yaml that actually starts the signer, is that commit missing?
ex:
https://github.com/degen-lab/stacks-blockchain-docker/blob/feat/add-signer-and-update-versions/compose-files/extra-services/proxy.yaml

i think you'll want another file like this for the signer unless the signer should run by default, in which case it can be added here: https://github.com/degen-lab/stacks-blockchain-docker/blob/feat/add-signer-and-update-versions/compose-files/common.yaml

@bowtiedbot
Copy link
Contributor Author

You're right, for some reason Github decided to ignore the signer files. I've added them now in the latest commit, thank you!

@wileyj
Copy link
Collaborator

wileyj commented Oct 25, 2024

thanks! seems good to me, but i won't be able to check this soon. i'll ping adriano, if he can get it running we'll merge it

Copy link

@aldur aldur left a comment

Choose a reason for hiding this comment

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

Thank you! I'll try running it on a clean environment and report back.

Meanwhile, I also asked @friedger if he can help testing it, since this closes #126.

Comment on lines +36 to +37
| `SIGNER_PRIVATE_KEY` | The private key of the signer, on mainnet. | |
| `TESTNET_SIGNER_PRIVATE_KEY` | The private key of the signer, on testnet. | |
Copy link

Choose a reason for hiding this comment

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

Just curious why two separate variables?

Copy link
Contributor

Choose a reason for hiding this comment

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

So we can run a testnet instance and a mainnet instance in the same environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because most users have different signer private keys on mainnet vs testnet. Users shouldn't worry about modifying the key every time they switch between a mainnet and a testnet instance, but have both set in env.

@@ -154,9 +155,10 @@ usage() {
log " -n|--network: [ mainnet | testnet | mocknet ]"
log " -a|--action: [ start | stop | logs | reset | upgrade | import | export | bns ]"
log " optional args:"
log " -f|--flags: [ proxy ]"
log " -f|--flags: [ signer,proxy ]"
Copy link

Choose a reason for hiding this comment

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

Can you also add how to run this on the readme?

Copy link
Contributor

@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.

I think my signer is running.

The beginning was a bit scary. signer couldn't start, stacks node was waiting on the signer.... eventually, it runs.

@@ -25,7 +25,7 @@ cp sample.env .env
### Seed chainstate from Hiro Archiver

Using data from the [Hiro Archiver](https://docs.hiro.so/hiro-archive) service, this script will download the latest files, extract them and restore the postgres data. \
_**Note**: it can take a long time to process the data, and you'll need at a minimum roughly 150GB of free space_
_**Note**: it can take a long time to process the data, and you'll need at a minimum roughly 350GB of free space_
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not related to signer, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's strictly from the node and API data. The node has about 250GB unarchived, while the API has 80.

[[events_observer]]
endpoint = "stacks-blockchain-api:3700"
events_keys = ["*"]
stacker = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This is optional for instances that choose to not run the signing service. What is the impact?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When starting without the signer, that line gets commented, along with the signer's events_observer line: https://github.com/degen-lab/stacks-blockchain-docker/blob/feat/add-signer-and-update-versions/manage.sh#L453-L463

Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@@ -33,3 +43,39 @@ amount = 10000000000000000
[[ustx_balance]]
address = "ST2TFVBMRPS5SSNP98DQKQ5JNB2B6NZM91C4K3P7B"
amount = 10000000000000000

[[burnchain.epochs]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these important?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the node has to know the epochs set by Hiro on their bootstrap node. These are taken from the Stacks Docs page with Sample Configuration Files. The testnet node would panic otherwise: https://docs.stacks.co/reference/sample-configuration-files#stacks-node-testnet-config

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

Comment on lines +36 to +37
| `SIGNER_PRIVATE_KEY` | The private key of the signer, on mainnet. | |
| `TESTNET_SIGNER_PRIVATE_KEY` | The private key of the signer, on testnet. | |
Copy link
Contributor

Choose a reason for hiding this comment

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

So we can run a testnet instance and a mainnet instance in the same environment?


[connection_options]
auth_token = "1234"
private_neighbors = false
Copy link
Contributor

Choose a reason for hiding this comment

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

why not using default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is so the node doesn't announce/accept neighbors that are behind private networks - leaving it as a default (true) might load the logs with messages that the connection between your node and the private node couldn't be established

db_path = "/root/stacks-signer/data/signer.sqlite"
auth_password = ""
stacks_private_key = ""
block_proposal_timeout_ms = 180000
Copy link
Contributor

Choose a reason for hiding this comment

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

We also want prometheus as a default, I think.

add command to run the environment with signer on README.md
@bowtiedbot
Copy link
Contributor Author

I've added metrics endpoints for node and signer, as well as how to run the environment with a signer directly on README.md in the latest commit.

@friedger
Copy link
Contributor

Could you add an option to remove stacks-blockchain-api ?
Maybe it is too much of an effort and we should create a fork?

@aldur
Copy link

aldur commented Oct 26, 2024

I think it's a reasonable thing to do, but I'm worried about scope creep on this PR. I'd suggest closing this and then iterating to disable the API in case.

@wileyj
Copy link
Collaborator

wileyj commented Oct 28, 2024

I think my signer is running.
The beginning was a bit scary. signer couldn't start, stacks node was waiting on the signer.... eventually, it runs.

good enough for me - i'll merge this, and we can figure out improvements/changes post 3.0 activation.
thanks for the PR!

@wileyj wileyj merged commit 9098ef2 into stacks-network:master Oct 28, 2024
1 check passed
@friedger
Copy link
Contributor

@wileyj this is not a solution that follows the recommendations for running a signer. They say that signers should run a stacks node without any other services like mock mining or stacks-api.

@wileyj
Copy link
Collaborator

wileyj commented Oct 30, 2024

fair point - in this case i think it's better to keep since it is optional as a flag (for the signer) and we'll tighten this repo up later on to disable the api if the signer is set to start.

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.

5 participants