-
Notifications
You must be signed in to change notification settings - Fork 389
refactor: add BzzAddress to ChainConfig and remove LookupERC20Address #5476
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
fba2633
98bbf7a
6760e92
f6e6847
d9c6903
599f33b
6fb543d
f49da5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // Copyright 2026 The Swarm Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style | ||
| // license that can be found in the LICENSE file. | ||
|
|
||
| package config_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/ethersphere/go-storage-incentives-abi/abi" | ||
|
|
||
| "github.com/ethersphere/bee/v2/pkg/config" | ||
| ) | ||
|
|
||
| func TestChainConfigBzzAddress(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| cfg config.ChainConfig | ||
| want common.Address | ||
| }{ | ||
| { | ||
| name: "mainnet", | ||
| cfg: config.Mainnet, | ||
| want: common.HexToAddress(abi.MainnetBzzTokenAddress), | ||
| }, | ||
| { | ||
| name: "testnet", | ||
| cfg: config.Testnet, | ||
| want: common.HexToAddress(abi.TestnetBzzTokenAddress), | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| if (tc.want == common.Address{}) { | ||
| t.Fatal("expected a non-zero bzz token address") | ||
| } | ||
| if tc.cfg.BzzAddress != tc.want { | ||
| t.Fatalf("got bzz address %s, want %s", tc.cfg.BzzAddress, tc.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestGetByChainIDBzzAddress(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| t.Run("known chain", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg, found := config.GetByChainID(config.Mainnet.ChainID) | ||
| if !found { | ||
| t.Fatal("expected mainnet to be a known chain") | ||
| } | ||
| if cfg.BzzAddress != common.HexToAddress(abi.MainnetBzzTokenAddress) { | ||
| t.Fatalf("got bzz address %s, want %s", cfg.BzzAddress, abi.MainnetBzzTokenAddress) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("unknown chain", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| cfg, found := config.GetByChainID(-1) | ||
| if found { | ||
| t.Fatal("expected unknown chain to be reported as not found") | ||
| } | ||
| if (cfg.BzzAddress != common.Address{}) { | ||
| t.Fatalf("expected zero bzz address for unknown chain, got %s", cfg.BzzAddress) | ||
| } | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,6 +139,7 @@ type Options struct { | |
| BlockchainRpcTLSTimeout time.Duration | ||
| BlockchainRpcIdleTimeout time.Duration | ||
| BlockchainRpcKeepalive time.Duration | ||
| BzzTokenAddress string | ||
| BlockProfile bool | ||
| BlockTime time.Duration | ||
| BlockSyncInterval uint64 | ||
|
|
@@ -539,56 +540,55 @@ func NewBee( | |
| } | ||
| } | ||
|
|
||
| if chainEnabled { | ||
| chequebookFactory, ferr := InitChequebookFactory(logger, chainBackend, chainID, transactionService, o.SwapFactoryAddress) | ||
| if o.SwapEnable && ferr != nil { | ||
| return nil, fmt.Errorf("init chequebook factory: %w", ferr) | ||
| } | ||
| chainCfg, found := config.GetByChainID(chainID) | ||
|
|
||
| if ferr == nil { | ||
| erc20Address, err := chequebookFactory.ERC20Address(ctx) | ||
| if err != nil { | ||
| if o.SwapEnable { | ||
| return nil, fmt.Errorf("factory fail: %w", err) | ||
| } | ||
| logger.Warning("unable to resolve ERC20 token address; BZZ balance will be unavailable via /wallet", "error", err) | ||
| } else { | ||
| erc20Service = erc20.New(transactionService, erc20Address) | ||
| } | ||
| } else { | ||
| logger.Warning("unable to init chequebook factory; BZZ balance will be unavailable via /wallet", "error", ferr) | ||
| bzzTokenAddress := chainCfg.BzzAddress | ||
| if o.BzzTokenAddress != "" { | ||
|
martinconic marked this conversation as resolved.
Outdated
|
||
| if !common.IsHexAddress(o.BzzTokenAddress) { | ||
| return nil, errors.New("malformed bzz token address") | ||
| } | ||
| bzzTokenAddress = common.HexToAddress(o.BzzTokenAddress) | ||
| } else if chainEnabled && bzzTokenAddress == (common.Address{}) { | ||
|
martinconic marked this conversation as resolved.
Outdated
|
||
| return nil, errors.New("no known bzz token address for this network; provide --bzz-token-address") | ||
| } | ||
|
|
||
| if o.SwapEnable { | ||
| if o.ChequebookEnable { | ||
| var err error | ||
| chequebookService, err = InitChequebookService( | ||
| ctx, | ||
| logger, | ||
| stateStore, | ||
| signer, | ||
| chainID, | ||
| chainBackend, | ||
| overlayEthAddress, | ||
| transactionService, | ||
| chequebookFactory, | ||
| o.SwapInitialDeposit, | ||
| erc20Service, | ||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("init chequebook service: %w", err) | ||
| } | ||
| } | ||
| if chainEnabled { | ||
| erc20Service = erc20.New(transactionService, bzzTokenAddress) | ||
| } | ||
|
Comment on lines
556
to
+561
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Full bzz token address verification can be moved here, not to have it partially before calling constructor and inside of it. But if chainEnabled {
if o.BzzTokenAddress != "" {
if !common.IsHexAddress(o.BzzTokenAddress) {
return nil, errors.New("malformed bzz token address")
}
bzzTokenAddress = common.HexToAddress(o.BzzTokenAddress)
}
if bzzTokenAddress == (common.Address{}) {
return nil, errors.New("no known bzz token address for this network; provide --bzz-token-address")
}
erc20Service = erc20.New(transactionService, bzzTokenAddress)
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the previous review round Elad asked specifically for the opposite — sanitize the address in cmd and pass a typed common.Address to NewBee rather than a raw string (done in 599f33b). That aligns it with how the other contract-address flags are validated and keeps the constructor free of string parsing. I'd prefer no to flip it back unless we settle on one convention — @acud, any objection to consolidating validation in node.go as @gacevicljubisa suggests, or keep it cmd-side?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have missed that comment. Most of verification is done inside the constructor, and my idea was to have it in one place and all can be inside this one if statement when chain is enabled. In any case no need to return error if chain is disabled. |
||
|
|
||
| chequeStore, cashoutService = initChequeStoreCashout( | ||
| if o.SwapEnable { | ||
| chequebookFactory, err := InitChequebookFactory(logger, chainBackend, chainID, transactionService, o.SwapFactoryAddress) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("init chequebook factory: %w", err) | ||
| } | ||
|
|
||
| if o.ChequebookEnable && chainEnabled { | ||
|
martinconic marked this conversation as resolved.
Outdated
|
||
| chequebookService, err = InitChequebookService( | ||
| ctx, | ||
| logger, | ||
| stateStore, | ||
| chainBackend, | ||
| chequebookFactory, | ||
| signer, | ||
| chainID, | ||
| chainBackend, | ||
| overlayEthAddress, | ||
| transactionService, | ||
| chequebookFactory, | ||
| o.SwapInitialDeposit, | ||
| erc20Service, | ||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("init chequebook service: %w", err) | ||
| } | ||
| } | ||
|
|
||
| chequeStore, cashoutService = initChequeStoreCashout( | ||
| stateStore, | ||
| chainBackend, | ||
| chequebookFactory, | ||
| chainID, | ||
| overlayEthAddress, | ||
| transactionService, | ||
| ) | ||
| } | ||
|
|
||
| lightNodes := lightnode.NewContainer(swarmAddress) | ||
|
|
@@ -706,7 +706,6 @@ func NewBee( | |
| eventListener postage.Listener | ||
| ) | ||
|
|
||
| chainCfg, found := config.GetByChainID(chainID) | ||
| postageStampContractAddress, postageSyncStart := chainCfg.PostageStampAddress, chainCfg.PostageStampStartBlock | ||
| if o.PostageContractAddress != "" { | ||
| if !common.IsHexAddress(o.PostageContractAddress) { | ||
|
|
@@ -723,11 +722,6 @@ func NewBee( | |
|
|
||
| postageStampContractABI := abiutil.MustParseABI(chainCfg.PostageStampABI) | ||
|
|
||
| bzzTokenAddress, err := postagecontract.LookupERC20Address(ctx, transactionService, postageStampContractAddress, postageStampContractABI, chainEnabled) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("lookup erc20 postage address: %w", err) | ||
| } | ||
|
|
||
| // Compute gas limit for contract transactions: when TrxDebugMode is enabled, | ||
| // gas estimation is skipped and DefaultGasLimit is used for all contract calls. | ||
| var contractGasLimit uint64 | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.