Skip to content

Commit

Permalink
fix incorrect fee displayed in UI for user-prompt action to bump fees…
Browse files Browse the repository at this point in the history
…, minor improvement in the way stuck transactions are handled
  • Loading branch information
norwnd committed Dec 30, 2024
1 parent 730aea9 commit f21b5ca
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 45 deletions.
56 changes: 24 additions & 32 deletions client/asset/eth/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ const (

LiveEstimateFailedError = dex.ErrorKind("live gas estimate failed")

// txAgeOut is the amount of time after which we forego any tx
// synchronization efforts for unconfirmed pending txs.
txAgeOut = 2 * time.Hour
// txActionPromptFrequency is the amount of time we wait between prompting user to
// resolve certain transaction processing issues that require manual input.
txActionPromptFrequency = 5 * time.Minute
// stateUpdateTick is the minimum amount of time between checks for
// new block and updating of pending txs, counter-party redemptions and
// approval txs.
Expand Down Expand Up @@ -1181,15 +1181,10 @@ func nonceIsSane(pendingTxs []*extendedWalletTx, pendingNonceAt *big.Int) error
}
lastNonce = nonce
age := pendingTx.age()
// Only allow a handful of txs that we haven't been seen on-chain yet.
// Only allow a handful of txs that we haven't seen on-chain yet.
if age > stateUpdateTick*10 {
numNotIndexed++
}
if age >= txAgeOut {
// If any tx is unindexed and aged out, wait for user to fix it.
return fmt.Errorf("tx %s is aged out. waiting for user to take action", pendingTx.ID)
}

}
if numNotIndexed >= maxUnindexedTxs {
return fmt.Errorf("%d unindexed txs has reached the limit of %d", numNotIndexed, maxUnindexedTxs)
Expand Down Expand Up @@ -3754,24 +3749,21 @@ func (eth *ETHWallet) checkForNewBlocks(ctx context.Context) {
return
}
bestHash := bestHdr.Hash()
// This method is called frequently. Don't hold write lock
// unless tip has changed.
eth.tipMtx.RLock()

eth.tipMtx.Lock()
currentTipHash := eth.currentTip.Hash()
eth.tipMtx.RUnlock()
if currentTipHash == bestHash {
eth.tipMtx.Unlock()
return
}

eth.tipMtx.Lock()
prevTip := eth.currentTip
eth.currentTip = bestHdr
eth.tipMtx.Unlock()

eth.log.Tracef("tip change: %s (%s) => %s (%s)", prevTip.Number,
currentTipHash, bestHdr.Number, bestHash)
eth.log.Tracef("tip change: %s (%s) => %s (%s)", prevTip.Number, currentTipHash, bestHdr.Number, bestHash)

eth.checkPendingTxs()

for _, w := range eth.connectedWallets() {
w.checkFindRedemptions()
w.checkPendingApprovals()
Expand Down Expand Up @@ -4235,7 +4227,7 @@ func (w *assetWallet) balanceWithTxPool() (*Balance, error) {
func (w *ETHWallet) sendToAddr(addr common.Address, amt uint64, maxFeeRate, tipRate *big.Int) (tx *types.Transaction, err error) {

// Uncomment here and above to test actionTypeLostNonce.
// Also change txAgeOut to like 1 minute.
// Also change txActionPromptFrequency to like 1 minute.
// if nonceBorked.CompareAndSwap(false, true) {
// defer w.borkNonce(tx)
// }
Expand Down Expand Up @@ -4698,7 +4690,6 @@ func (w *baseWallet) checkPendingTxs() {
w.log.Tracef("Checked %d pending txs. Finalized %d", nPending, nPending-len(w.pendingTxs))
}()
}

}

// keepFromIndex will be the index of the first un-finalized tx.
Expand Down Expand Up @@ -4745,17 +4736,17 @@ func (w *baseWallet) checkPendingTxs() {
if pendingTx.Confirmed || pendingTx.BlockNumber > 0 {
continue
}
if time.Since(pendingTx.actionIgnored) < txAgeOut {
// They asked us to keep waiting.
continue
if time.Since(pendingTx.lastActionProcessed) < txActionPromptFrequency {
continue // user asked us to keep waiting
}
age := pendingTx.age()
// i < lastConfirmed means unconfirmed nonce < a confirmed nonce.
if (i < lastConfirmed) ||
w.confirmedNonceAt.Cmp(pendingTx.Nonce) > 0 ||
(age >= txAgeOut && pendingTx.Receipt == nil && !pendingTx.indexed) {

// The tx in our records wasn't accepted. Where's the right one?
if (i < lastConfirmed) || w.confirmedNonceAt.Cmp(pendingTx.Nonce) > 0 {
// The tx in our records wasn't accepted. Seems like it has been replaced by another
// transaction with the same nonce, asking the user if he knows what transaction
// corresponds to this nonce is the only way to mend this transaction.
// This can only happen if user is doing something advanced (like sending transactions
// from this wallet using another software), or if there is a bug in our code - so
// it's fine to keep asking him to resolve it.
req := newLostNonceNote(*pendingTx.WalletTransaction, pendingTx.Nonce.Uint64())
pendingTx.actionRequested = true
w.requestAction(actionTypeLostNonce, pendingTx.ID, req, pendingTx.TokenID)
Expand All @@ -4775,11 +4766,12 @@ func (w *baseWallet) checkPendingTxs() {
txCap := tx.GasFeeCap()
baseRate, tipRate, err := w.currentNetworkFees(w.ctx)
if err != nil {
w.log.Errorf("Error getting base fee: %w", err)
w.log.Errorf("Error getting base fee: %v", err)
continue
}
if txCap.Cmp(baseRate) < 0 {
maxFees := new(big.Int).Add(tipRate, new(big.Int).Mul(baseRate, big.NewInt(2)))
// Propose new fees that are more suitable to current network conditions.
maxFees := new(big.Int).Add(tipRate, baseRate)
maxFees.Mul(maxFees, new(big.Int).SetUint64(tx.Gas()))
req := newLowFeeNote(*pendingTx.WalletTransaction, dexeth.WeiToGweiCeil(maxFees))
pendingTx.actionRequested = true
Expand Down Expand Up @@ -4904,7 +4896,7 @@ func (w *assetWallet) userActionBumpFees(actionB []byte) error {
}
return w.amendPendingTx(action.TxID, func(txHash common.Hash, tx *types.Transaction, pendingTx *extendedWalletTx, idx int) error {
if !*action.Bump {
pendingTx.actionIgnored = time.Now()
pendingTx.lastActionProcessed = time.Now()
return nil
}

Expand Down Expand Up @@ -4969,7 +4961,7 @@ func (w *assetWallet) userActionNonceReplacement(actionB []byte) error {
abandon := *action.Abandon
if !abandon && action.ReplacementID == "" { // keep waiting
return w.amendPendingTx(action.TxID, func(_ common.Hash, _ *types.Transaction, pendingTx *extendedWalletTx, idx int) error {
pendingTx.actionIgnored = time.Now()
pendingTx.lastActionProcessed = time.Now()
return nil
})
}
Expand Down
4 changes: 2 additions & 2 deletions client/asset/eth/eth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ func TestCheckPendingTxs(t *testing.T) {
finalizedStamp := now - txConfsNeededToConfirm*10
rebroadcastable := now - 300
mature := now - 600 // able to send actions
agedOut := now - uint64(txAgeOut.Seconds()) - 1
agedOut := now - uint64(txActionPromptFrequency.Seconds()) - 1

val := dexeth.GweiToWei(1)
extendedTx := func(nonce, blockNum, blockStamp, submissionStamp uint64) *extendedWalletTx {
Expand Down Expand Up @@ -889,7 +889,7 @@ func TestTakeAction(t *testing.T) {
if tx.GasTipCap().Uint64() != 0 {
t.Fatal("The fee was bumped. The fee shouldn't have been bumped.")
}
if pendingTx.actionIgnored.IsZero() {
if pendingTx.lastActionProcessed.IsZero() {
t.Fatalf("The ignore time wasn't reset")
}
if len(eth.pendingTxs) != 1 {
Expand Down
16 changes: 8 additions & 8 deletions client/asset/eth/txdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ type extendedWalletTx struct {
// ActionRequiredNote.
AssumedLost bool `json:"assumedLost,omitempty"`

txHash common.Hash
lastCheck uint64
savedToDB bool
lastBroadcast time.Time
lastFeeCheck time.Time
actionRequested bool
actionIgnored time.Time
indexed bool
txHash common.Hash
lastCheck uint64
savedToDB bool
lastBroadcast time.Time
lastFeeCheck time.Time
actionRequested bool
lastActionProcessed time.Time
indexed bool
}

func (t *extendedWalletTx) age() time.Duration {
Expand Down
4 changes: 2 additions & 2 deletions client/webserver/site/src/html/bodybuilder.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,14 @@
</td>
</tr>
<tr>
<td>[[[Fees]]]</td>
<td>Current Fees (too low)</td>
<td>
<span data-tmpl="feeAmount"></span>
<span data-tmpl="feeUnit" class="fs15 grey"></span>
</td>
</tr>
<tr data-tmpl="newFeesRow" class="d-hide">
<td>New Fees</td>
<td>Suggested New Fees</td>
<td>
<span data-tmpl="newFees"></span>
<span data-tmpl="newFeesUnit" class="fs15 grey"></span>
Expand Down
2 changes: 1 addition & 1 deletion client/webserver/site/src/js/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ export default class Application {
switch (req.actionID) {
case 'tooCheap': {
Doc.show(tmpl.newFeesRow)
tmpl.newFees.textContent = Doc.formatCoinAtom(n.tx.fees, parentUI)
tmpl.newFees.textContent = Doc.formatCoinAtom(n.newFees, parentUI)
tmpl.newFeesUnit.textContent = parentUI.conventional.unit
break
}
Expand Down

0 comments on commit f21b5ca

Please sign in to comment.