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

Jump to bottom

client/asset/eth: less provider check spam #2159

Merged
merged 3 commits into from
Feb 23, 2023
Merged

client/asset/eth: less provider check spam #2159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2674488
client/asset/eth: silence provider check spam
chappjc Feb 22, 2023
3675ba0
client/asset/eth: fix unable to write compliant_providers.json
chappjc Feb 6, 2023
3428fd9
remove some "geth" mentions in logs, a couple other silly things
chappjc Feb 22, 2023
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
41 changes: 20 additions & 21 deletions client/asset/eth/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"fmt"
"math"
"math/big"
"os"
"os/user"
"path/filepath"
"sort"
Expand Down Expand Up @@ -589,6 +590,11 @@ func createWallet(createWalletParams *asset.CreateWalletParams, skipConnect bool
// defer node.Close()
// return importKeyToNode(node, privateKey, createWalletParams.Pass)
case walletTypeRPC:
// Make the wallet dir if it does not exist, otherwise we may fail to
// write the compliant_providers.json file.
if err := os.MkdirAll(walletDir, 0700); err != nil {
return err
}

// Check that we can connect to all endpoints.
providerDef := createWalletParams.Settings[providersKey]
Expand Down Expand Up @@ -672,7 +678,7 @@ func NewWallet(assetCFG *asset.WalletConfig, logger dex.Logger, net dex.Network)

aw := &assetWallet{
baseWallet: eth,
log: logger.SubLogger("ETH"),
log: logger,
assetID: BipID,
tipChange: assetCFG.TipChange,
findRedemptionReqs: make(map[[32]byte]*findRedemptionRequest),
Expand Down Expand Up @@ -792,15 +798,15 @@ func (w *ETHWallet) Connect(ctx context.Context) (_ *sync.WaitGroup, err error)
// Initialize the best block.
bestHdr, err := w.node.bestHeader(ctx)
if err != nil {
return nil, fmt.Errorf("error getting best block hash from geth: %w", err)
return nil, fmt.Errorf("error getting best block hash: %w", err)
}
w.tipMtx.Lock()
w.currentTip = bestHdr
w.tipMtx.Unlock()
height := w.currentTip.Number
// NOTE: We should be using the tipAtConnect to set Progress in SyncStatus.
atomic.StoreInt64(&w.tipAtConnect, height.Int64())
w.log.Infof("Connected to geth, at height %d", height)
w.log.Infof("Connected to eth (%s), at height %d", w.walletType, height)

w.connected.Store(true)

Expand Down Expand Up @@ -2917,28 +2923,24 @@ func (eth *baseWallet) SyncStatus() (bool, float32, error) {
if err != nil {
return false, 0, err
}
checkHeaderTime := func() (bool, error) {
checkHeaderTime := func() bool {
// Time in the header is in seconds.
timeDiff := time.Now().Unix() - int64(tipTime)
if timeDiff > dexeth.MaxBlockInterval && eth.net != dex.Simnet {
eth.log.Infof("Time since last eth block (%d sec) exceeds %d sec."+
"Assuming not in sync. Ensure your computer's system clock "+
"is correct.", timeDiff, dexeth.MaxBlockInterval)
return false, nil
return false
}
return true, nil
return true
}
if prog.HighestBlock != 0 {
// HighestBlock was set. This means syncing started and is
// finished if CurrentBlock is higher. CurrentBlock will
// continue to go up even if we are not in a syncing state.
// HighestBlock will not.
if prog.CurrentBlock >= prog.HighestBlock {
fresh, err := checkHeaderTime()
if err != nil {
return false, 0, err
}
if !fresh {
if fresh := checkHeaderTime(); !fresh {
return false, 0, nil
}
return eth.node.peerCount() > 0, 1.0, nil
Expand All @@ -2959,11 +2961,7 @@ func (eth *baseWallet) SyncStatus() (bool, float32, error) {
// syncing state. In order to discern if syncing has begun when
// HighestBlock is not set, check that the best header came in under
// dexeth.MaxBlockInterval and guess.
fresh, err := checkHeaderTime()
if err != nil {
return false, 0, err
}
if !fresh {
if fresh := checkHeaderTime(); !fresh {
return false, 0, nil
}
return eth.node.peerCount() > 0, 1.0, nil
Expand Down Expand Up @@ -3495,7 +3493,7 @@ func (w *assetWallet) checkUnconfirmedRedemption(secretHash common.Hash, contrac
// confirmRedemptionWithoutMonitoredTx checks the confirmation status of a
// redemption transaction. It is called when a monitored tx cannot be
// found. The main difference between the regular path and this one is that
// when geth can also not find the transaction, instead of resubmitting an
// when we can also not find the transaction, instead of resubmitting an
// entire redemption batch, a new transaction containing only the swap we are
// searching for will be created.
func (w *assetWallet) confirmRedemptionWithoutMonitoredTx(txHash common.Hash, redemption *asset.Redemption, feeWallet *assetWallet) (*asset.ConfirmRedemptionStatus, error) {
Expand All @@ -3511,7 +3509,7 @@ func (w *assetWallet) confirmRedemptionWithoutMonitoredTx(txHash common.Hash, re

tx, txBlock, err := w.node.getTransaction(w.ctx, txHash)
if errors.Is(err, asset.CoinNotFoundError) {
w.log.Errorf("ConfirmRedemption: geth could not find tx: %s", txHash)
w.log.Errorf("ConfirmRedemption: could not find tx: %s", txHash)
swapIsRedeemed, err := w.swapIsRedeemed(secretHash, contractVer)
if err != nil {
return nil, err
Expand All @@ -3520,7 +3518,7 @@ func (w *assetWallet) confirmRedemptionWithoutMonitoredTx(txHash common.Hash, re
return confStatus(txConfsNeededToConfirm, txHash), nil
}

// If geth cannot find the transaction, and it also wasn't among the
// If we cannot find the transaction, and it also wasn't among the
// monitored txs, we will resubmit the swap individually.
txs, _, _, err := w.Redeem(&asset.RedeemForm{
Redemptions: []*asset.Redemption{redemption},
Expand Down Expand Up @@ -3610,7 +3608,7 @@ func (w *assetWallet) confirmRedemption(coinID dex.Bytes, redemption *asset.Rede
tx, txBlock, err := w.node.getTransaction(w.ctx, txHash)
if errors.Is(err, asset.CoinNotFoundError) {
if blocksSinceSubmission > 2 {
w.log.Errorf("ConfirmRedemption: geth could not find tx: %s", txHash)
w.log.Errorf("ConfirmRedemption: could not find tx: %s", txHash)
}

if blocksSinceSubmission < blocksToWaitBeforeCoinNotFound {
Expand Down Expand Up @@ -3941,7 +3939,8 @@ func (w *assetWallet) estimateTransferGas(val uint64) (gas uint64, err error) {

// redeem redeems a swap contract. Any on-chain failure, such as this secret not
// matching the hash, will not cause this to error.
func (w *assetWallet) redeem(ctx context.Context, assetID uint32, redemptions []*asset.Redemption, maxFeeRate, gasLimit uint64, contractVer uint32, nonceOverride *uint64) (tx *types.Transaction, err error) {
func (w *assetWallet) redeem(ctx context.Context, assetID uint32 /* ?? */, redemptions []*asset.Redemption,
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know what this input parameter is about. Should it have been used or was this copy pasta from other methods like this?

maxFeeRate, gasLimit uint64, contractVer uint32, nonceOverride *uint64) (tx *types.Transaction, err error) {
w.nonceSendMtx.Lock()
defer w.nonceSendMtx.Unlock()
var nonce *big.Int
Expand Down
40 changes: 15 additions & 25 deletions client/asset/eth/multirpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"decred.org/dcrdex/dex"
"decred.org/dcrdex/dex/networks/erc20"
dexeth "decred.org/dcrdex/dex/networks/eth"
"github.com/decred/slog"
"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -697,7 +696,7 @@ func createAndCheckProviders(ctx context.Context, walletDir string, endpoints []
if len(providers) != len(unknownEndpoints) {
return providersErr(providers)
}
if err := checkProvidersCompliance(ctx, providers, net, log); err != nil {
if err := checkProvidersCompliance(ctx, providers, net, dex.Disabled /* logger is for testing only */); err != nil {
Copy link
Member Author

@chappjc chappjc Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Could do this when calling createAndCheckProviders, but that's only used in product code, whereas the tests use checkProvidersCompliance directly.
Happy to change.

EDIT: There are some logs up there we probably would want.

return err
}
}
Expand Down Expand Up @@ -1400,7 +1399,9 @@ type rpcTest struct {

// newCompatibilityTests returns a list of RPC tests to run to determine API
// compatibility.
func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Logger) []*rpcTest {
func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log dex.Logger) []*rpcTest {
// NOTE: The logger is intended for use the execution of the compatibility
// tests, and it will generally be dex.Disabled in production.
var (
// Vitalik's address from https://twitter.com/VitalikButerin/status/1050126908589887488
mainnetAddr = common.HexToAddress("0xab5801a7d398351b8be11c439e05c5b3259aec9b")
Expand Down Expand Up @@ -1440,15 +1441,15 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
readIt := func(path string) string {
b, err := os.ReadFile(path)
if err != nil {
log.Errorf("Problem reading simnet testing file: %v", err)
panic(fmt.Sprintf("Problem reading simnet testing file %q: %v", path, err))
}
return strings.TrimRight(string(b), "\r\n")
return strings.TrimSpace(string(b)) // mainly the trailing "\r\n"
}
usdc = common.HexToAddress(readIt(tContractFile))
txHash = common.HexToHash(readIt(tTxHashFile))
blockHash = common.HexToHash(readIt(tBlockHashFile))
default:
log.Errorf("Unknown net %v in compatibility tests. Testing data not initiated.", net)
default: // caller should have checked though
panic(fmt.Sprintf("Unknown net %v in compatibility tests. Testing data not initiated.", net))
Comment on lines +1451 to +1452
Copy link
Member Author

@chappjc chappjc Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In the product code, there are probably 30 places it would panic before getting here anyway. It's virtually impossible to get an unrecognized dex.Network here with the product where it's checked first thing on app bring-up.

}

return []*rpcTest{
Expand Down Expand Up @@ -1487,7 +1488,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
if err != nil {
return err
}
log.Info("#### Retreived tip cap:", tipCap)
log.Debugf("#### Retrieved tip cap: %d gwei", dexeth.WeiToGwei(tipCap))
return nil
},
},
Expand All @@ -1498,7 +1499,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
if err != nil {
return err
}
log.Infof("#### Balance retrieved: %.9f", float64(dexeth.WeiToGwei(bal))/1e9)
log.Debugf("#### Balance retrieved: %.9f", float64(dexeth.WeiToGwei(bal))/1e9)
return nil
},
},
Expand All @@ -1509,7 +1510,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
if err != nil {
return err
}
log.Infof("#### %d bytes of USDC contract retrieved", len(code))
log.Debugf("#### %d bytes of USDC contract retrieved", len(code))
return nil
},
},
Expand All @@ -1530,7 +1531,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
// I guess we would need to unpack the results. I don't really
// know how to interpret these, but I'm really just looking for
// a request error.
log.Info("#### USDC balanceOf result:", bal, "wei")
log.Debug("#### USDC balanceOf result:", dexeth.WeiToGwei(bal), "gwei")
return nil
},
},
Expand All @@ -1541,18 +1542,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
if err != nil {
return err
}
log.Infof("#### Chain ID: %d", chainID)
return nil
},
},
{
name: "PendingNonceAt",
f: func(ctx context.Context, p *provider) error {
n, err := p.ec.PendingNonceAt(ctx, addr)
if err != nil {
return err
}
log.Infof("#### Pending nonce: %d", n)
log.Debugf("#### Chain ID: %d", chainID)
return nil
},
},
Expand All @@ -1567,7 +1557,7 @@ func newCompatibilityTests(cb bind.ContractBackend, net dex.Network, log slog.Lo
if rpcTx.BlockNumber != nil {
h = *rpcTx.BlockNumber
}
log.Infof("#### RPC Tx is nil? %t, block number: %q", rpcTx.tx == nil, h)
log.Debugf("#### RPC Tx is nil? %t, block number: %q", rpcTx.tx == nil, h)
return nil
},
},
Expand All @@ -1587,7 +1577,7 @@ func domain(host string) string {
// requires by sending a series of requests and verifying the responses. If a
// provider is found to be compliant, their domain name is added to a list and
// stored in a file on disk so that future checks can be short-circuited.
func checkProvidersCompliance(ctx context.Context, providers []*provider, net dex.Network, log slog.Logger) error {
func checkProvidersCompliance(ctx context.Context, providers []*provider, net dex.Network, log dex.Logger) error {
for _, p := range providers {
// Need to run API tests on this endpoint.
for _, t := range newCompatibilityTests(p.ec, net, log) {
Expand Down
4 changes: 2 additions & 2 deletions client/core/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2534,7 +2534,7 @@ func (c *Core) createSeededWallet(assetID uint32, crypter encrypt.Crypter, form
Settings: form.Config,
DataDir: c.assetDataDirectory(assetID),
Net: c.net,
Logger: c.log.SubLogger("CREATE"),
Logger: c.log.SubLogger(unbip(assetID)),
}); err != nil {
return nil, fmt.Errorf("Error creating wallet: %w", err)
}
Expand Down Expand Up @@ -2959,7 +2959,7 @@ func (c *Core) RecoverWallet(assetID uint32, appPW []byte, force bool) error {
Settings: dbWallet.Settings,
DataDir: c.assetDataDirectory(assetID),
Net: c.net,
Logger: c.log.SubLogger("CREATE"),
Logger: c.log.SubLogger(unbip(assetID)),
}); err != nil {
return fmt.Errorf("error creating wallet: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions server/asset/eth/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func unconnectedETH(logger dex.Logger, net dex.Network) (*ETHBackend, error) {
baseLogger: logger,
tokens: make(map[uint32]*TokenBackend),
},
log: logger.SubLogger("ETH"),
log: logger,
contractAddr: contractAddr,
blockChans: make(map[chan *asset.BlockUpdate]struct{}),
initTxSize: uint32(dexeth.InitGas(1, ethContractVersion)),
Expand Down Expand Up @@ -332,7 +332,7 @@ func (eth *ETHBackend) Connect(ctx context.Context) (*sync.WaitGroup, error) {
bn, err := eth.node.blockNumber(ctx)
if err != nil {
cancelNodeContext()
return nil, fmt.Errorf("error getting best block header from geth: %w", err)
return nil, fmt.Errorf("error getting best block header: %w", err)
}
eth.baseBackend.bestHeight = bn

Expand Down Expand Up @@ -715,7 +715,7 @@ func (eth *ETHBackend) poll(ctx context.Context) {
}
bn, err := eth.node.blockNumber(ctx)
if err != nil {
send(fmt.Errorf("error getting best block header from geth: %w", err))
send(fmt.Errorf("error getting best block header: %w", err))
return
}
if bn == eth.bestHeight {
Expand Down