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

Custom Fallback TOML Config #15617

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tall-falcons-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#added the ability to define a fallback.toml override config using CL_CHAIN_DEFAULTS env var
134 changes: 87 additions & 47 deletions core/chains/evm/config/toml/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package toml
import (
"bytes"
"embed"
"errors"
"fmt"
"io"
"log"
"os"
"path/filepath"
Expand All @@ -19,7 +19,6 @@ import (
)

var (

//go:embed defaults/*.toml
defaultsFS embed.FS
fallback Chain
Expand All @@ -33,48 +32,68 @@ var (
)

func init() {
// read the defaults first
// read all default configs
initReadDefaults()

// check for and apply any overrides
initApplyEVMOverrides()
Comment on lines +35 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this indirection is adding value. Please consider setting the global vars from this func, one way or another.
https://github.com/smartcontractkit/chainlink/pull/15617/files#r1883872492

}

var (
errRead = errors.New("error reading file")
errDecode = errors.New("error in TOML decoding")
errMissingChainID = errors.New("missing ChainID")
errNonNilChainID = errors.New("fallback ChainID must be nil")
errFallbackConfig = errors.New("fallback config")
Comment on lines +43 to +47
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 all single use? I think the indirection makes it harder to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I guess errFallbackConfig is actually checked for

)

func initReadDefaults() {
// read the defaults first
fes, err := defaultsFS.ReadDir("defaults")
if err != nil {
log.Fatalf("failed to read defaults/: %v", err)
log.Fatalf("failed to read defaults: %v", err)
}

for _, fe := range fes {
path := filepath.Join("defaults", fe.Name())
b, err2 := defaultsFS.ReadFile(path)
if err2 != nil {
log.Fatalf("failed to read %q: %v", path, err2)
if fe.IsDir() {
// Skip directories
continue
}
var config = struct {
ChainID *big.Big
Chain
}{}

if err3 := cconfig.DecodeTOML(bytes.NewReader(b), &config); err3 != nil {
log.Fatalf("failed to decode %q: %v", path, err3)
}
if fe.Name() == "fallback.toml" {
if config.ChainID != nil {
log.Fatalf("fallback ChainID must be nil, not: %s", config.ChainID)
// read the file to bytes
path := filepath.Join("defaults", fe.Name())
chainID, chain, err := readConfig(path, defaultsFS.ReadFile, fe.Name() == "fallback.toml")
if err != nil {
if errors.Is(err, errFallbackConfig) {
fallback = chain

continue
}
fallback = config.Chain
continue
}
if config.ChainID == nil {
log.Fatalf("missing ChainID: %s", path)

log.Fatal(err.Error())
}
DefaultIDs = append(DefaultIDs, config.ChainID)
id := config.ChainID.String()

// add ChainID to set of default IDs
DefaultIDs = append(DefaultIDs, chainID)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is important not to separate these global var assignments (defaults and defaultNames too) from their declarations.

Copy link
Contributor

@jmank88 jmank88 Dec 13, 2024

Choose a reason for hiding this comment

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

If you want to use helpers, then how about returning values to set from init()?


// ChainID as a default should not be duplicated
id := chainID.String()
if _, ok := defaults[id]; ok {
log.Fatalf("%q contains duplicate ChainID: %s", path, id)
}
defaults[id] = config.Chain

// set default lookups
defaults[id] = chain
defaultNames[id] = strings.ReplaceAll(strings.TrimSuffix(fe.Name(), ".toml"), "_", " ")
}

// sort default IDs in numeric order
slices.SortFunc(DefaultIDs, func(a, b *big.Big) int {
return a.Cmp(b)
})
}

func initApplyEVMOverrides() {
// read the custom defaults overrides
dir := env.CustomDefaults.Get()
if dir == "" {
Expand All @@ -98,39 +117,60 @@ func init() {
continue
}

// read the file to bytes
path := evmDir + "/" + entry.Name()
file, err := os.Open(path)
chainID, chain, err := readConfig(path, os.ReadFile, entry.Name() == "fallback.toml")
if err != nil {
log.Fatalf("error opening file (name: %v) in custom defaults override directory: %v", entry.Name(), err)
if errors.Is(err, errFallbackConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We already know when it is a fallback config, because we just passed the boolean arg to readConfig. Let's use that logic directly instead of hacking errors.

fallback = chain

continue
}

log.Fatalf("custom defaults override failure (%s): %s", entry.Name(), err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Fatalf("custom defaults override failure (%s): %s", entry.Name(), err.Error())
log.Fatalf("custom defaults override failure (%s): %s", entry.Name(), err)

}

// Read file contents
b, err := io.ReadAll(file)
file.Close()
if err != nil {
log.Fatalf("error reading file (name: %v) contents in custom defaults override directory: %v", entry.Name(), err)
// ChainID as a default should not be duplicated
id := chainID.String()
if _, ok := customDefaults[id]; ok {
log.Fatalf("%q contains duplicate ChainID: %s", path, id)
}

var config = struct {
ChainID *big.Big
Chain
}{}
// set default lookups
customDefaults[id] = chain
}
}

if err := cconfig.DecodeTOML(bytes.NewReader(b), &config); err != nil {
log.Fatalf("failed to decode %q in custom defaults override directory: %v", path, err)
}
func readConfig(path string, reader func(name string) ([]byte, error), fallback bool) (*big.Big, Chain, error) {
bts, err := reader(path)
if err != nil {
return nil, Chain{}, fmt.Errorf("%w: %s", errRead, err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, Chain{}, fmt.Errorf("%w: %s", errRead, err.Error())
return nil, Chain{}, fmt.Errorf("%w: %w", errRead, err)

}

if config.ChainID == nil {
log.Fatalf("missing ChainID in: %s in custom defaults override directory. exiting", path)
}
var config = struct {
ChainID *big.Big
Chain
}{}

id := config.ChainID.String()
// decode from toml to a chain config
if err := cconfig.DecodeTOML(bytes.NewReader(bts), &config); err != nil {
return nil, Chain{}, fmt.Errorf("%w %s: %s", errDecode, path, err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, Chain{}, fmt.Errorf("%w %s: %s", errDecode, path, err.Error())
return nil, Chain{}, fmt.Errorf("%w %s: %w", errDecode, path, err)

}

if _, ok := customDefaults[id]; ok {
log.Fatalf("%q contains duplicate ChainID: %s", path, id)
if fallback {
if config.ChainID != nil {
return nil, Chain{}, fmt.Errorf("%w: found: %s", errNonNilChainID, config.ChainID)
}
customDefaults[id] = config.Chain

return nil, config.Chain, errFallbackConfig
Comment on lines +160 to +165
Copy link
Contributor

@jmank88 jmank88 Dec 13, 2024

Choose a reason for hiding this comment

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

We should not be using a custom error to communicate back to the caller when there is not an error, and it seems unnecessary in this case since they passed it in to us in the first place.

}

// ensure ChainID is set
if config.ChainID == nil {
return nil, Chain{}, fmt.Errorf("%w: %s", errMissingChainID, path)
}

return config.ChainID, config.Chain, nil
}

// DefaultsNamed returns the default Chain values, optionally for the given chainID, as well as a name if the chainID is known.
Expand Down
Loading
Loading