feat: refactor dymint config to support loading configuration from file #326
Conversation
Codecov Report
@@ Coverage Diff @@
## main #326 +/- ##
==========================================
+ Coverage 53.68% 53.82% +0.13%
==========================================
Files 80 80
Lines 12901 12876 -25
==========================================
+ Hits 6926 6930 +4
+ Misses 5032 5013 -19
+ Partials 943 933 -10
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…rt-loading-configuration-from-file
conv/config_test.go
Outdated
| @@ -12,14 +12,18 @@ import ( | |||
| func TestGetNodeConfig(t *testing.T) { | |||
| t.Parallel() | |||
|
|
|||
| validCosmos := "127.0.0.1:1234" | |||
| validOptimint := "/ip4/127.0.0.1/tcp/1234" | |||
There was a problem hiding this comment.
copied it from conv/addr_test.go
state/executor.go
Outdated
| func NewBlockExecutor(proposerAddress []byte, namespaceID string, chainID string, mempool mempool.Mempool, proxyApp proxy.AppConns, eventBus *tmtypes.EventBus, logger log.Logger) *BlockExecutor { | ||
| bytes, err := hex.DecodeString(namespaceID) | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
is the error for the rollapp operator indicative in case a wrong string is supplied?
There was a problem hiding this comment.
i'll add some logging
settlement/config.go
Outdated
| // Config for the DymensionLayerClient | ||
| type Config struct { | ||
| KeyringBackend cosmosaccount.KeyringBackend `json:"keyring_backend"` |
There was a problem hiding this comment.
why did you remove KeyringBackend?
There was a problem hiding this comment.
we only support test for the sequencer
no reason to expose it as a configuration parameter if it's not supported
There was a problem hiding this comment.
are you sure? In the cosmosclient we init I see this keyring backends supported:
type KeyringBackend string
const (
// KeyringTest is the test keyring backend. With this backend, your keys will be
// stored under your app's data dir,
KeyringTest KeyringBackend = "test"
// KeyringOS is the OS keyring backend. with this backend, your keys will be
// stored in your operating system's secured keyring.
KeyringOS KeyringBackend = "os"
// KeyringMemory is in memory keyring backend, your keys will be stored in application memory.
KeyringMemory KeyringBackend = "memory"
)
either way we'll for sure need to support more keyring backends in the near future (specifically ledger) so not sure what's the motivation for removing it
There was a problem hiding this comment.
yeah but the sequencer can't use anything else as it will request signing approval for each submission.
anyway I leave it in the configuration
…-dymint-config-to-support-loading-configuration-from-file
mtsitrin
deleted the
mtsitrin/325-refactor-dymint-config-to-support-loading-configuration-from-file
branch
namespaceIDto string so it will be easier to set in conf filesettlement.LayerClienttosettlement.LayerIto indicate it's an interfacePR Standards
Opening a pull request should be able to meet the following requirements
For Author:
godoccommentsFor Reviewer:
After reviewer approval: