pbft signing threshold config from CLI #19
Conversation
use the --pbft-threshold parameter
src/exec/Main.hs
Outdated
| Opt.long "pbft-threshold" <> | ||
| Opt.metavar "DOUBLE" <> | ||
| -- FIXME ouroboros-consensus should export defaultPBftSignatureThreshold | ||
| -- That'd be better than putting this magic number here. |
There was a problem hiding this comment.
Alternatively, I could make this an optional parameter with no default, and give Nothing to protocolInfoByron. Any strong arguments in either favour?
There was a problem hiding this comment.
You can make this a Maybe field, since protocolInfoByron accepts a maybe, and it uses the default if you pass nothing. That'd be the simplest.
src/exec/Main.hs
Outdated
| Opt.long "pbft-threshold" <> | ||
| Opt.metavar "DOUBLE" <> | ||
| -- FIXME ouroboros-consensus should export defaultPBftSignatureThreshold | ||
| -- That'd be better than putting this magic number here. |
There was a problem hiding this comment.
You can make this a Maybe field, since protocolInfoByron accepts a maybe, and it uses the default if you pass nothing. That'd be the simplest.
src/exec/Main.hs
Outdated
| @@ -507,7 +522,7 @@ main = do | |||
| (Cardano.ApplicationName (fromString "cardano-byron-proxy")) 2 | |||
| protocolInfo = protocolInfoByron | |||
| newGenesisConfig | |||
| Nothing -- Default signature threshold. | |||
| (Just (bpoPBftSignatureThreshold bpo)) | |||
There was a problem hiding this comment.
Pass the maybe though here, so it'll use the default in the nothing case.
|
Agreed that having an optional parameter is better than a default value here. Only downside is that the default remains obscure (it's not even exported). |
Rather than have a default in CLI The default is still there, by way of byronProtocolInfo from ouroboros-consensus.
|
bors r+ |
use the --pbft-threshold parameter