-
Notifications
You must be signed in to change notification settings - Fork 170
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
Parameterize genesis and config through testnet command flags #115
Conversation
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.
Looks good!
@@ -93,6 +115,12 @@ Example: | |||
cmd.Flags().String(flagNodeDirPrefix, "node", "Prefix the directory name for each node with (node results in node0, node1, ...)") | |||
cmd.Flags().String(flagNodeDaemonHome, "babylond", "Home directory of the node's daemon configuration") | |||
cmd.Flags().String(flagStartingIPAddress, "192.168.0.1", "Starting IP address (192.168.0.1 results in persistent peers list [email protected]:46656, [email protected]:46656, ...)") | |||
cmd.Flags().String(flagBtcNetwork, string(bbn.BtcSimnet), "Bitcoin network to use. Available networks: simnet, testnet, mainnet") | |||
cmd.Flags().String(flagBtcCheckpointTag, string(txformat.TestTag), "Tag to use for Bitcoin checkpoints. Available tags: BBT, BBN") | |||
cmd.Flags().Uint64(flagEpochInterval, 10, "Number of blocks between epochs. Must be more than 0.") |
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.
How often does it create a block by default? Isn't it something like 2 seconds?
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 think yes. Added this as a default parameter since the default param for the genesis module is 10.
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.
Then maybe the default should reflect the time we actually want our epoch to last, which I assume is not 20 seconds.
@@ -93,6 +115,12 @@ Example: | |||
cmd.Flags().String(flagNodeDirPrefix, "node", "Prefix the directory name for each node with (node results in node0, node1, ...)") | |||
cmd.Flags().String(flagNodeDaemonHome, "babylond", "Home directory of the node's daemon configuration") | |||
cmd.Flags().String(flagStartingIPAddress, "192.168.0.1", "Starting IP address (192.168.0.1 results in persistent peers list [email protected]:46656, [email protected]:46656, ...)") | |||
cmd.Flags().String(flagBtcNetwork, string(bbn.BtcSimnet), "Bitcoin network to use. Available networks: simnet, testnet, mainnet") | |||
cmd.Flags().String(flagBtcCheckpointTag, string(txformat.TestTag), "Tag to use for Bitcoin checkpoints. Available tags: BBT, BBN") |
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.
These tags make me think that if you use BBT, if you want to restart your testnet, and you use the same BBT tag again, the clients will be confused that Bitcoin already has checkpoints for BBT; the new testnet will look like a long range attack.
Maybe a counter like BBT01
, hoping there won't be more than 99 testnets? Or maybe a different letters like BBA
, BBB
, BBC
. In any case I would not hardcode too much that it can be BBT or BBN.
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.
Currently I think it is not allowed to have a tag other than that. Is that right @KonradStaniec ?
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.
Yep we can have only BBN
or BBT
for now and we can use any tag on any network.
Main use case being, that on any btc network you can put test tags (BBT
) just to test babylon nodes with them so you can run babylon testnet against bitcoin mainnet and not have tags mix with each other. Then when running babylon mainnet against btc mainnet tag won't mix together.
if we would like multiple testnets against some long living btc nets (mainnet/testnet) then probably we would need more tags. Imo only requirement here is that tag have max 3bytes as this greatly reduces encoding/decoding complexity.
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 see, although I think what Akosh suggests would be super useful, especially when using the testnet. However, even if we allow the testnet flag to be larger (e.g. BBT01-BBT99), we still restrict ourselves to the number of testnets that we can deploy using the BTC testnet. We need to specify a format for this case.
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.
Does the checkpoint have a version, just in case you change your mind about the content?
Regarding the counter: it could be an actual u8
, so you have up to 256 nets with the same tag.
My point is that the BBT
checkpoints will not disappear from the BTC testnet at the same time as you want to restart testnet, so if everything works well you might not even be able to run a testnet a second time. So it's not a long-lived testnet I'm worried about but rather the short lived ones.
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.
Yep, agreed. I suggest we extend the checkpoint format to allow for multiple testnets.
Add flags for:
Also, modify the configuration to allow CORS requests etc. as specified by the requirements in the explorer repo
The main goal of this PR is to enable the automatic generation of testnet files for different settings: