Skip to content
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
6 changes: 2 additions & 4 deletions pkg/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,14 @@ func (s *Service) mountBusinessDebug() {
))

handle("/wallet", web.ChainHandlers(
s.checkChequebookAvailability,
s.checkSwapAvailability,
s.checkChainAvailability,
web.FinalHandler(jsonhttp.MethodHandler{
"GET": http.HandlerFunc(s.walletHandler),
}),
))

handle("/wallet/withdraw/{coin}", web.ChainHandlers(
s.checkChequebookAvailability,
s.checkSwapAvailability,
s.checkChainAvailability,
web.FinalHandler(jsonhttp.MethodHandler{
"POST": web.ChainHandlers(
s.gasConfigMiddleware("wallet withdraw"),
Expand Down
8 changes: 4 additions & 4 deletions pkg/api/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,8 @@ func TestEndpointOptions(t *testing.T) {
{"/chequebook/address", []string{"GET"}, http.StatusNoContent},
{"/chequebook/deposit", []string{"POST"}, http.StatusNoContent},
{"/chequebook/withdraw", []string{"POST"}, http.StatusNoContent},
{"/wallet", nil, http.StatusForbidden},
{"/wallet/withdraw/{coin}", nil, http.StatusForbidden},
{"/wallet", []string{"GET"}, http.StatusNoContent},
{"/wallet/withdraw/{coin}", []string{"POST"}, http.StatusNoContent},
{"/stamps", []string{"GET"}, http.StatusNoContent},
{"/stamps/{batch_id}", []string{"GET"}, http.StatusNoContent},
{"/stamps/{batch_id}/buckets", []string{"GET"}, http.StatusNoContent},
Expand Down Expand Up @@ -385,8 +385,8 @@ func TestEndpointOptions(t *testing.T) {
{"/chequebook/address", nil, http.StatusForbidden},
{"/chequebook/deposit", nil, http.StatusForbidden},
{"/chequebook/withdraw", nil, http.StatusForbidden},
{"/wallet", nil, http.StatusForbidden},
{"/wallet/withdraw/{coin}", nil, http.StatusForbidden},
{"/wallet", []string{"GET"}, http.StatusNoContent},
{"/wallet/withdraw/{coin}", []string{"POST"}, http.StatusNoContent},
{"/stamps", []string{"GET"}, http.StatusNoContent},
{"/stamps/{batch_id}", []string{"GET"}, http.StatusNoContent},
{"/stamps/{batch_id}/buckets", []string{"GET"}, http.StatusNoContent},
Expand Down
19 changes: 13 additions & 6 deletions pkg/api/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ func (s *Service) walletHandler(w http.ResponseWriter, r *http.Request) {
return
}

bzz, err := s.erc20Service.BalanceOf(r.Context(), s.ethereumAddress)
if err != nil {
logger.Debug("unable to acquire erc20 balance", "error", err)
logger.Error(nil, "unable to acquire erc20 balance")
jsonhttp.InternalServerError(w, "unable to acquire erc20 balance")
return
bzz := new(big.Int)
if s.erc20Service != nil {
bzz, err = s.erc20Service.BalanceOf(r.Context(), s.ethereumAddress)
if err != nil {
logger.Debug("unable to acquire erc20 balance", "error", err)
logger.Error(nil, "unable to acquire erc20 balance")
jsonhttp.InternalServerError(w, "unable to acquire erc20 balance")
return
}
}

jsonhttp.OK(w, walletResponse{
Expand Down Expand Up @@ -95,6 +98,10 @@ func (s *Service) walletWithdrawHandler(w http.ResponseWriter, r *http.Request)
}

if bzz {
if s.erc20Service == nil {
jsonhttp.ServiceUnavailable(w, "BZZ token address unavailable")
return
}
currentBalance, err := s.erc20Service.BalanceOf(r.Context(), s.ethereumAddress)
if err != nil {
logger.Error(err, "unable to get balance")
Expand Down
52 changes: 52 additions & 0 deletions pkg/api/wallet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,58 @@ func TestWallet(t *testing.T) {
Code: 500,
}))
})

t.Run("swap disabled", func(t *testing.T) {
t.Parallel()

srv, _, _, _ := newTestServer(t, testServerOptions{
SwapDisabled: true,
Erc20Opts: []erc20mock.Option{
erc20mock.WithBalanceOfFunc(func(ctx context.Context, address common.Address) (*big.Int, error) {
return big.NewInt(10000000000000000), nil
}),
},
BackendOpts: []backendmock.Option{
backendmock.WithBalanceAt(func(ctx context.Context, address common.Address, block *big.Int) (*big.Int, error) {
return big.NewInt(2000000000000000000), nil
}),
},
})

jsonhttptest.Request(t, srv, http.MethodGet, "/wallet", http.StatusOK,
jsonhttptest.WithExpectedJSONResponse(api.WalletResponse{
BZZ: bigint.Wrap(big.NewInt(10000000000000000)),
NativeToken: bigint.Wrap(big.NewInt(2000000000000000000)),
ChainID: 1,
}),
)
})

t.Run("chequebook disabled", func(t *testing.T) {
t.Parallel()

srv, _, _, _ := newTestServer(t, testServerOptions{
ChequebookDisabled: true,
Erc20Opts: []erc20mock.Option{
erc20mock.WithBalanceOfFunc(func(ctx context.Context, address common.Address) (*big.Int, error) {
return big.NewInt(10000000000000000), nil
}),
},
BackendOpts: []backendmock.Option{
backendmock.WithBalanceAt(func(ctx context.Context, address common.Address, block *big.Int) (*big.Int, error) {
return big.NewInt(2000000000000000000), nil
}),
},
})

jsonhttptest.Request(t, srv, http.MethodGet, "/wallet", http.StatusOK,
jsonhttptest.WithExpectedJSONResponse(api.WalletResponse{
BZZ: bigint.Wrap(big.NewInt(10000000000000000)),
NativeToken: bigint.Wrap(big.NewInt(2000000000000000000)),
ChainID: 1,
}),
)
})
}

func TestWalletWithdraw(t *testing.T) {
Expand Down
72 changes: 43 additions & 29 deletions pkg/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ func NewBee(

chainEnabled := isChainEnabled(o, o.BlockchainRpcEndpoint, logger)

if o.SwapEnable && !chainEnabled {
return nil, errors.New("swap is enabled but the chain backend is not; provide --blockchain-rpc-endpoint or disable swap")
}

var batchStore postage.Storer = new(postage.NoOpBatchStore)
var evictFn func([]byte) error

Expand Down Expand Up @@ -535,46 +539,56 @@ func NewBee(
}
}

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 chainEnabled {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider extracting this whole chainEnabled block into a private helper, there are to many conditionals, it is hard to follow...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, and we would need some refactoring in general not only for chainEnabled here, chainEnabled and other options are hard to follow in multiple areas. This should be a different PR and some effort is already ongoing in #5471

chequebookFactory, ferr := InitChequebookFactory(logger, chainBackend, chainID, transactionService, o.SwapFactoryAddress)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When swap is disabled, the factory is only being initialized so we can read the BZZ token address. Could we use postagecontract.LookupERC20Address above this instead?
LookupERC20Address resolves the same token from the postage stamp contract, and would let us skip InitChequebookFactory entirely on the swap disabled path?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — postagecontract.LookupERC20Address is in fact already called below in the postage setup (node.go:726) to resolve bzzTokenAddress, so the factory's ERC20Address here is redundant on the swap-disabled path. Reusing it would let us drop the factory init entirely when swap is off. The wrinkle is ordering: erc20Service is built here, while bzzTokenAddress only resolves ~180 lines lower, so switching cleanly means reordering the erc20 init relative to the postage block. I'd rather keep this PR scoped to the /wallet gating fix and do the lookup reuse as a follow-up so the reordering gets its own review. WDYT?

if o.SwapEnable && ferr != nil {
return nil, fmt.Errorf("init chequebook factory: %w", ferr)
}

erc20Address, err := chequebookFactory.ERC20Address(ctx)
if err != nil {
return nil, fmt.Errorf("factory fail: %w", err)
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)
}

erc20Service = erc20.New(transactionService, erc20Address)
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 o.ChequebookEnable && chainEnabled {
chequebookService, err = InitChequebookService(
ctx,
logger,
chequeStore, cashoutService = initChequeStoreCashout(
stateStore,
signer,
chainID,
chainBackend,
chequebookFactory,
chainID,
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)
Expand Down
Loading