beacon-kit icon indicating copy to clipboard operation
beacon-kit copied to clipboard

chore(config): Fix error handling of empty `suggested-fee-recipient`

Open calbera opened this issue 8 months ago • 1 comments

Closes #2699

With an empty suggested-fee-recipient will instead fail to startup with this error message now:

panic: error calling provider github.com/berachain/beacon-kit/node-core/components.ProvideConfig (github.com/berachain/beacon-kit/node-core/components/config.go:36): 1 error(s) decoding:
	
	* error decoding 'beacon-kit.payload-builder.suggested-fee-recipient': empty hex string

goroutine 1 [running]:
github.com/berachain/beacon-kit/node-core/builder.(*NodeBuilder).Build(0x1400000f938, 0x14000d917d0, {0x1024c7e00, 0x14000522528}, {0x0?, 0x0?}, 0x14000a08780, {0x102491300, 0x14000a78380})
	github.com/berachain/beacon-kit/node-core/builder/builder.go:98 +0x438
github.com/berachain/beacon-kit/cli/commands/server.StartCmdWithOptions.func1(0x140006bec08, {0x101a46d7e?, 0x4?, 0x101a46c5e?})
	github.com/berachain/beacon-kit/cli/commands/server/start.go:99 +0xbc
github.com/spf13/cobra.(*Command).execute(0x140006bec08, {0x14000a6e6c0, 0xc, 0xc})
	github.com/spf13/[email protected]/command.go:985 +0x834
github.com/spf13/cobra.(*Command).ExecuteC(0x140006a2308)
	github.com/spf13/[email protected]/command.go:1117 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/[email protected]/command.go:1041
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	github.com/spf13/[email protected]/command.go:1034
github.com/berachain/beacon-kit/cli/commands/server/cmd.Execute(0x140006a2308, {0x0, 0x0}, {0x1400014edb0, 0x17})
	github.com/berachain/beacon-kit/cli/commands/server/cmd/execute.go:61 +0x270
github.com/berachain/beacon-kit/cli/commands.(*Root).Run(0x14000699e00?, {0x1400014edb0?, 0x101aacd8e?})
	github.com/berachain/beacon-kit/cli/commands/root.go:92 +0x34
main.run()
	./main.go:77 +0x2bc
main.main()
	./main.go:82 +0x1c
make: *** [start] Error 2

Would suggest keeping this behavior. i.e. if an operator goes out of their way to make this value empty (the default will set it to "0x0000000000000000000000000000000000000000"), it should fail imo. Lmk your thoughts @camembera.

calbera avatar Apr 22 '25 22:04 calbera

Codecov Report

:x: Patch coverage is 50.00000% with 11 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 60.89%. Comparing base (9369bd6) to head (924b457). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cli/commands/genesis/deposit.go 0.00% 4 Missing :warning:
config/viper/parser.go 40.00% 2 Missing and 1 partial :warning:
primitives/common/execution.go 57.14% 2 Missing and 1 partial :warning:
cli/utils/parser/validator.go 0.00% 1 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2700      +/-   ##
==========================================
- Coverage   60.92%   60.89%   -0.03%     
==========================================
  Files         358      358              
  Lines       16871    16883      +12     
==========================================
+ Hits        10278    10281       +3     
- Misses       5799     5806       +7     
- Partials      794      796       +2     
Files with missing lines Coverage Δ
config/spec/devnet.go 100.00% <100.00%> (ø)
config/spec/mainnet.go 100.00% <100.00%> (ø)
cli/utils/parser/validator.go 13.33% <0.00%> (ø)
config/viper/parser.go 76.47% <40.00%> (-4.78%) :arrow_down:
primitives/common/execution.go 84.48% <57.14%> (-3.98%) :arrow_down:
cli/commands/genesis/deposit.go 40.67% <0.00%> (-1.07%) :arrow_down:

... and 1 file with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 22 '25 22:04 codecov[bot]

i think 0x0 is a sensible value for this field for ordinary deployments like RPC and infra

wherever the block producer starts, it should check there's a fee recipient or refuse to start

Yes exactly. Leaving it or forcing it as empty will fail fast. If you are non-validator node, simply set it to the 0 address "0x0000000000000000000000000000000000000000".

calbera avatar Aug 28 '25 18:08 calbera

LGTM. If you decide to go out of your way to burn your tips that's getting what you ask for

camembera avatar Aug 28 '25 19:08 camembera