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.

Sign up for GitHub

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

fix: runtime panic #2836

Merged
merged 5 commits into from
Mar 7, 2022
Merged

fix: runtime panic #2836

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d4aa73
fix: runtime panic
notanatol Mar 4, 2022
bfe05a1
fix: revert override
notanatol Mar 4, 2022
2a17719
fix: debugapi chain flag
notanatol Mar 5, 2022
39de79e
fix: remove flag
notanatol Mar 7, 2022
706287e
fix: debug api fix
notanatol Mar 7, 2022
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
1 change: 0 additions & 1 deletion cmd/bee/cmd/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func (c *command) initDeployCmd() error {
chequebookFactory,
swapInitialDeposit,
deployGasPrice,
true,
)
if err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions pkg/debugapi/chequebook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethersphere/bee/pkg/bigint"
"github.com/ethersphere/bee/pkg/jsonhttp"
"github.com/ethersphere/bee/pkg/postage/postagecontract"
"github.com/ethersphere/bee/pkg/sctx"
"github.com/ethersphere/bee/pkg/settlement/swap"
"github.com/ethersphere/bee/pkg/settlement/swap/chequebook"
Expand Down Expand Up @@ -68,6 +69,12 @@ type chequebookLastChequesResponse struct {

func (s *Service) chequebookBalanceHandler(w http.ResponseWriter, r *http.Request) {
balance, err := s.chequebook.Balance(r.Context())
if errors.Is(err, postagecontract.ErrChainDisabled) {
jsonhttp.MethodNotAllowed(w, err)
s.logger.Debugf("debug api: chequebook balance: %v", err)
s.logger.Error("debug api: cannot get chequebook balance")
return
}
if err != nil {
jsonhttp.InternalServerError(w, errChequebookBalance)
s.logger.Debugf("debug api: chequebook balance: %v", err)
Expand Down
57 changes: 56 additions & 1 deletion pkg/debugapi/debugapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package debugapi

import (
"context"
"crypto/ecdsa"
"math/big"
"net/http"
Expand Down Expand Up @@ -114,7 +115,7 @@ func New(publicKey, pssPublicKey ecdsa.PublicKey, ethereumAddress common.Address
// Configure injects required dependencies and configuration parameters and
// constructs HTTP routes that depend on them. It is intended and safe to call
// this method only once.
func (s *Service) Configure(overlay swarm.Address, p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, lightNodes *lightnode.Container, storer storage.Storer, tags *tags.Tags, accounting accounting.Interface, pseudosettle settlement.Interface, swapEnabled bool, chequebookEnabled bool, swap swap.Interface, chequebook chequebook.Service, batchStore postage.Storer, post postage.Service, postageContract postagecontract.Interface, traverser traversal.Traverser) {
func (s *Service) Configure(overlay swarm.Address, p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, lightNodes *lightnode.Container, storer storage.Storer, tags *tags.Tags, accounting accounting.Interface, pseudosettle settlement.Interface, swapEnabled bool, chequebookEnabled bool, swap swap.Interface, chequebook chequebook.Service, batchStore postage.Storer, post postage.Service, postageContract postagecontract.Interface, traverser traversal.Traverser, chainEnabled bool) {
s.p2p = p2p
s.pingpong = pingpong
s.topologyDriver = topologyDriver
Expand All @@ -125,6 +126,9 @@ func (s *Service) Configure(overlay swarm.Address, p2p p2p.DebugService, pingpon
s.chequebook = chequebook
s.swapEnabled = swapEnabled
s.swap = swap
if !chainEnabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

This extra parameter can be avoided by either initialising swap to noOpSwap before passing here or maybe checking if the swap == nil.

s.swap = new(noOpSwap)
}
s.lightNodes = lightNodes
s.batchStore = batchStore
s.pseudosettle = pseudosettle
Expand All @@ -145,3 +149,54 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) {

h.ServeHTTP(w, r)
}

type noOpSwap struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

So I went through the APIs to see how they are handled. Seems like there are a few errors like ErrNoChequebook which are being handled. I wonder if we should retain the errors here instead of returning empty responses? If API expects to send No Chequebook error if there is no chequebook, then this would sort of break that no?

}

func (*noOpSwap) TotalSent(peer swarm.Address) (totalSent *big.Int, err error) {
return big.NewInt(0), nil
}

// TotalReceived returns the total amount received from a peer
func (*noOpSwap) TotalReceived(peer swarm.Address) (totalSent *big.Int, err error) {
return big.NewInt(0), nil
}

// SettlementsSent returns sent settlements for each individual known peer
func (*noOpSwap) SettlementsSent() (map[string]*big.Int, error) {
return nil, nil
}

// SettlementsReceived returns received settlements for each individual known peer
func (*noOpSwap) SettlementsReceived() (map[string]*big.Int, error) {
return nil, nil
}

func (*noOpSwap) LastSentCheque(peer swarm.Address) (*chequebook.SignedCheque, error) {
return nil, nil
}

// LastSentCheques returns the list of last sent cheques for all peers
func (*noOpSwap) LastSentCheques() (map[string]*chequebook.SignedCheque, error) {
return nil, nil
}

// LastReceivedCheque returns the last received cheque for the peer
func (*noOpSwap) LastReceivedCheque(peer swarm.Address) (*chequebook.SignedCheque, error) {
return nil, nil
}

// LastReceivedCheques returns the list of last received cheques for all peers
func (*noOpSwap) LastReceivedCheques() (map[string]*chequebook.SignedCheque, error) {
return nil, nil
}

// CashCheque sends a cashing transaction for the last cheque of the peer
func (*noOpSwap) CashCheque(ctx context.Context, peer swarm.Address) (common.Hash, error) {
return common.Hash{}, nil
}

// CashoutStatus gets the status of the latest cashout transaction for the peers chequebook
func (*noOpSwap) CashoutStatus(ctx context.Context, peer swarm.Address) (*chequebook.CashoutStatus, error) {
return nil, nil
}
4 changes: 2 additions & 2 deletions pkg/debugapi/debugapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func newTestServer(t *testing.T, o testServerOptions) *testServer {
transaction := transactionmock.New(o.TransactionOpts...)
ln := lightnode.NewContainer(o.Overlay)
s := debugapi.New(o.PublicKey, o.PSSPublicKey, o.EthereumAddress, logging.New(io.Discard, 0), nil, o.CORSAllowedOrigins, big.NewInt(2), transaction, false, nil, false, debugapi.FullMode)
s.Configure(o.Overlay, o.P2P, o.Pingpong, topologyDriver, ln, o.Storer, o.Tags, acc, settlement, true, true, swapserv, chequebook, o.BatchStore, o.Post, o.PostageContract, o.Traverser)
s.Configure(o.Overlay, o.P2P, o.Pingpong, topologyDriver, ln, o.Storer, o.Tags, acc, settlement, true, true, swapserv, chequebook, o.BatchStore, o.Post, o.PostageContract, o.Traverser, true)
ts := httptest.NewServer(s)
t.Cleanup(ts.Close)

Expand Down Expand Up @@ -190,7 +190,7 @@ func TestServer_Configure(t *testing.T) {
}),
)

s.Configure(o.Overlay, o.P2P, o.Pingpong, topologyDriver, ln, o.Storer, o.Tags, acc, settlement, true, true, swapserv, chequebook, nil, mockpost.New(), nil, nil)
s.Configure(o.Overlay, o.P2P, o.Pingpong, topologyDriver, ln, o.Storer, o.Tags, acc, settlement, true, true, swapserv, chequebook, nil, mockpost.New(), nil, nil, true)

testBasicRouter(t, client)
jsonhttptest.Request(t, client, http.MethodGet, "/readiness", http.StatusOK,
Expand Down
22 changes: 9 additions & 13 deletions pkg/node/chain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/ethersphere/bee/pkg/crypto"
"github.com/ethersphere/bee/pkg/logging"
"github.com/ethersphere/bee/pkg/p2p/libp2p"
"github.com/ethersphere/bee/pkg/postage/postagecontract"
"github.com/ethersphere/bee/pkg/sctx"
"github.com/ethersphere/bee/pkg/settlement"
"github.com/ethersphere/bee/pkg/settlement/swap"
Expand Down Expand Up @@ -160,12 +161,7 @@ func InitChequebookService(
chequebookFactory chequebook.Factory,
initialDeposit string,
deployGasPrice string,
chainEnabled bool,
) (chequebook.Service, error) {
if !chainEnabled {
return new(noOpChequebookService), nil
}

chequeSigner := chequebook.NewChequeSigner(signer, chainID)

deposit, ok := new(big.Int).SetString(initialDeposit, 10)
Expand Down Expand Up @@ -347,31 +343,31 @@ func GetTxNextBlock(ctx context.Context, logger logging.Logger, backend transact
type noOpChequebookService struct{}

func (m *noOpChequebookService) Deposit(context.Context, *big.Int) (hash common.Hash, err error) {
return hash, errors.New("chain disabled")
return hash, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) Withdraw(context.Context, *big.Int) (hash common.Hash, err error) {
return hash, errors.New("chain disabled")
return hash, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) WaitForDeposit(context.Context, common.Hash) error {
return errors.New("chain disabled")
return postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) Balance(context.Context) (*big.Int, error) {
return nil, errors.New("chain disabled")
return nil, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) AvailableBalance(context.Context) (*big.Int, error) {
return nil, errors.New("chain disabled")
return nil, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) Address() common.Address {
return common.Address{}
}
func (m *noOpChequebookService) Issue(context.Context, common.Address, *big.Int, chequebook.SendChequeFunc) (*big.Int, error) {
return nil, errors.New("chain disabled")
return nil, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) LastCheque(common.Address) (*chequebook.SignedCheque, error) {
return nil, errors.New("chain disabled")
return nil, postagecontract.ErrChainDisabled
}
func (m *noOpChequebookService) LastCheques() (map[common.Address]*chequebook.SignedCheque, error) {
return nil, errors.New("chain disabled")
return nil, postagecontract.ErrChainDisabled
}

// noOpChainBackend is a noOp implementation for transaction.Backend interface.
Expand Down
2 changes: 1 addition & 1 deletion pkg/node/devnode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func NewDevBee(logger logging.Logger, o *DevOptions) (b *DevBee, err error) {
)

// inject dependencies and configure full debug api http path routes
debugAPIService.Configure(swarmAddress, p2ps, pingPong, kad, lightNodes, storer, tagService, acc, pseudoset, true, true, mockSwap, mockChequebook, batchStore, post, postageContract, traversalService)
debugAPIService.Configure(swarmAddress, p2ps, pingPong, kad, lightNodes, storer, tagService, acc, pseudoset, true, true, mockSwap, mockChequebook, batchStore, post, postageContract, traversalService, false)
}

return b, nil
Expand Down
7 changes: 3 additions & 4 deletions pkg/node/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func NewBee(addr string, publicKey *ecdsa.PublicKey, signer crypto.Signer, netwo
transactionService transaction.Service
transactionMonitor transaction.Monitor
chequebookFactory chequebook.Factory
chequebookService chequebook.Service
chequebookService chequebook.Service = new(noOpChequebookService)
chequeStore chequebook.ChequeStore
cashoutService chequebook.CashoutService
pollingInterval = time.Duration(o.BlockTime) * time.Second
Expand Down Expand Up @@ -343,7 +343,7 @@ func NewBee(addr string, publicKey *ecdsa.PublicKey, signer crypto.Signer, netwo
return nil, fmt.Errorf("factory fail: %w", err)
}

if o.ChequebookEnable {
if o.ChequebookEnable && chainEnabled {
chequebookService, err = InitChequebookService(
p2pCtx,
logger,
Expand All @@ -356,7 +356,6 @@ func NewBee(addr string, publicKey *ecdsa.PublicKey, signer crypto.Signer, netwo
chequebookFactory,
o.SwapInitialDeposit,
o.DeployGasPrice,
chainEnabled,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -870,7 +869,7 @@ func NewBee(addr string, publicKey *ecdsa.PublicKey, signer crypto.Signer, netwo
debugAPIService.MustRegisterMetrics(chainSyncer.Metrics()...)
}
// inject dependencies and configure full debug api http path routes
debugAPIService.Configure(swarmAddress, p2ps, pingPong, kad, lightNodes, storer, tagService, acc, pseudosettleService, o.SwapEnable, o.ChequebookEnable, swapService, chequebookService, batchStore, post, postageContractService, traversalService)
debugAPIService.Configure(swarmAddress, p2ps, pingPong, kad, lightNodes, storer, tagService, acc, pseudosettleService, o.SwapEnable, o.ChequebookEnable, swapService, chequebookService, batchStore, post, postageContractService, traversalService, chainEnabled)
}

if err := kad.Start(p2pCtx); err != nil {
Expand Down