-
Notifications
You must be signed in to change notification settings - Fork 23
Conversation
Lift and shift the run-random-beacon doc into something formatted for keep ecdsa. I realize there is more context that's needed here but I wanted to get this out so other folks can contribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding some initial thoughts about this document. I will add another review once I go through and perform the actual install steps on a couple systems. @pantsme#2124 on Discord btw.
|
||
- It is paramount that Keep nodes remain available to the Keep Network. We strongly encourage a | ||
stable and redundant internet connection. | ||
- A connection to a production grade self-hosted or third party Ethereum node deployment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a good idea to link/suggest Infura/Cloudflare here as a third-party Ethereum endpoint or link to Geth/Parity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've purposely tried to be a bit obtuse here so folks don't just pick what we have listed. Will think on this a bit.
docs/run-keep-ecdsa.adoc
Outdated
export KEEP_CLIENT_CONFIG_DIR=$(pwd)/config | ||
export KEEP_CLIENT_PERSISTENCE_DIR=$(pwd)/persistence | ||
|
||
docker run -dit \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason to use the -it
options here if using -d
. You are essentially telling Docker to open an interactive terminal but detach from it.
docker run -dit \ | |
docker run -d \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason to use the
-it
options here if using-d
.
This is a weird one. Even in detached mode if you set the entrypoint
to bash
or sh
you need the pseudo-terminal to keep the container running in detached mode, hence the -dit
. To your point this is kind of a holdover from docker run
scripts I use for local testing, where I frequently --entrypoint /bin/sh
. I should be able to remove this here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, that makes sense. I'm just being picky here really.
docs/run-keep-ecdsa.adoc
Outdated
=== Run Image | ||
This is a sample run command for illustration purposes only. | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this works on .adoc files, but you might be able to add bash after the gate open to get syntax highlighting on the commands within this code block.
``` | |
``` bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should, though I don’t know if the interstitial space will mess it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will give it a shot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't render but
[source,bash]
----
code
----
Seems to work.
docs/run-keep-ecdsa.adoc
Outdated
|
||
==== Sample | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add toml after start of code fence for syntax highlighting.
``` | |
``` toml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will try!
|n1-standard-4 | ||
|
||
|AWS | ||
|m5.xlarge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c5.xlarge
is a 4x8 machine and costs about $18 less per month than the m5.xlarge
as a suggestion to keep it cheaper on users. I do not know the specs you are going off of, I am just using the specs of a self-hosted being 4x4 so my guess is that CPU is more important than memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right that CPU is more important here. My specs are based off more general application such as being a Kube Node, where having the available memory for other components is relevant. We should probably capture both cases here, if you're running on a VM directly or as part of a multi-tenant setup such as Kube.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm cheap mostly is why I brought this up. I know a lot of people are cheap too so mostly trying to make it so that someone doesn't run a node because of the cost.
|
||
``` | ||
|
||
==== Parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to think about a better table layout for these parameters. The non-uniform size of each of them is hard to read. Might want to make it 5 columns wide and just include the left-most column header currently an entire new column all the way down. Then you'd be uniform.
Something like this?
[%header,cols=5*]
|===
|Type
|Description
|Default
|Required
|ethereum
|URL
|The Ethereum host your keep-ecdsa will connect to. Websocket protocol/port.
|""
|Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can see what it looks like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at how this renders on our docs page: https://docs.keep.network/run-random-beacon.html#_application, and they line up. I guess this is a product of how GH renders adoc files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting ^ is a different document, but formatted the same way.
@afmsavage Thanks a lot for the feedback. Going through these now. |
Here we clarify the port protocol so there is no confusion for folks during setup. Co-authored-by: Antonio Savage <[email protected]>
…work/keep-ecdsa into sthompson22/docs/run-keep-ecdsa
- Remove the -it from the run args. This is only applicable when you're setting the entrypoint to bash or sh when in detached mode. - s/CLIENT/ECDSA/g. This is a copy/paste leftover from the beacon docs. - Set entrypoint. This overrides the default entrypoint defined in the Dockerfile which may clash with the start options you're passing to the command.
Without the small font there bootstrap peers render on multiple lines in docs.
Ready for another pass. |
…m-beacon-updates Docs: run-random-beacon Update A few housekeeping items here: 1. docker run command cleanup. Here we set an entrypoint to explicitly override the ENTRYPOINT set in the Dockerfile. Without it you will have two --config flags set on the running process running the command provided in this doc. Somehow we've turned out ok here but it's caused some confusion with testers. Also removed the -it arguments from the command as they're not strictly required unless you're overriding ENTRYPOINT with bash or sh. 2. Some syntax highlighting motivated by the comments in keep-network/keep-ecdsa#396 3. Small text for bootstrap peers. docs.keep.network renders the peers on multiple lines without this option set.
docs/run-keep-ecdsa.adoc
Outdated
|
||
=== Authorizations | ||
Before operator is considered as eligible for work selection, authorizer appointed during the delegation needs to review | ||
and authorize TBTCSystem smart contract. Smart contracts can be authorized using KEEP token dashboard. Authorized operator contracts may slash or seize tokens in case of operator's misbehavior. Authorized operator contracts are set in `SanctionedApplications.Addresses` in your Keep ECDSA config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two authorizations that have to be done:
- authorizer has to authorize
BondedECDSAKeepFactory
operator contract inTokenStaking
contract - it lets the factory to slash tokens on misbehaviour and makes the operator eligible for work selection - authorizer has to provide a secondary authorization for the sortition pool created for the particular application to operate on the bonds stored in
KeepBonding
contract.
If we do not upgrade the system, there will be only one Tokenstaking
contract, KeepBonding
contract, and only one BondedECDSAKeepFactory
. The factory creates a sortition pool per application. So authorizer is de facto authorizing the application to operate on bonds, just indirectly.
Both actions will be available in the dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pdyraga tried to distill that into a couple bullets. Let me know what needs tweaking.
This aligns better with keep-client, the name of the core client, vs keep-ecdsa which doesn't indicate the client nature.
Also include address of TBTCSystem and its sortition pool for sanctioned applications list on Ropsten.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mais oui.
Drafting for visibility and other contributors to add info
=======
The intended audience for this document is anyone who is interested in running their own Keep ECDSA client.
Here we provide a semi-detailed overview of mainnet/testnet context, operational considerations, and app/logging/network configurations for running a Keep ECDSA client.
This document is meant to be living, especially as we work through helping folks onboard in Discord. It's important that we're identifying gaps through those Discord interactions and updating this doc accordingly.
The information here certainly isn't exhaustive, on this pass we're aiming to cover the most critical considerations for running a client.
ref: #391