Skip to content

Commit

Permalink
fix transactor_forGasEstimate lock. +QuoteDetails event
Browse files Browse the repository at this point in the history
  • Loading branch information
parodime committed Jan 31, 2025
1 parent 7b62df2 commit 73442fa
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 106 deletions.
114 changes: 41 additions & 73 deletions ethergo/submitter/submitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"math/big"
"reflect"
"runtime"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -388,9 +389,6 @@ func (t *txSubmitterImpl) SubmitTransaction(parentCtx context.Context, chainID *
// this also prevents a bug in the caller from breaking our lock
transactor.Nonce = new(big.Int).Add(new(big.Int).SetUint64(math.MaxUint64), big.NewInt(1))

//tmpdebug
fmt.Printf("SubmitTransaction>setGasPrice\n")

err = t.setGasPrice(ctx, chainClient, transactor, chainID, nil)
if err != nil {
span.AddEvent("could not set gas price", trace.WithAttributes(attribute.String("error", err.Error())))
Expand Down Expand Up @@ -422,64 +420,59 @@ func (t *txSubmitterImpl) SubmitTransaction(parentCtx context.Context, chainID *
return parentTransactor.Signer(address, transaction)
}

//tmpdebug
fmt.Printf("test ver 7\n")

// if dynamic gas estimation is not enabled, use cfg var gas_estimate as a gas limit default and do not run a pre-flight simulation
// since we do not need it to determine proper gas units
if !t.config.GetDynamicGasEstimate(int(chainID.Uint64())) {

Check failure on line 425 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)

//tmpdebug
fmt.Printf("Using Default \n")

transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))

Check failure on line 426 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)
} else {

// //tmpdebug
// fmt.Printf("SubmitTransaction>forGasEst call \n")
// deepcopy the real transactor so we can use it for simulation
transactor_forGasEstimate := copyTransactOpts(transactor)

Check failure on line 430 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

var-naming: don't use underscores in Go names; var transactor_forGasEstimate should be transactorForGasEstimate (revive)

// transactor_forGasEstimate := copyTransactOpts(transactor)
// override the signer func for our simulation/estimation with a version that does not lock the nonce,
// which would othewrise cause a deadlock with the following *actual* transactor
transactor_forGasEstimate.Signer = func(address common.Address, transaction *types.Transaction) (_ *types.Transaction, err error) {

// transactor_forGasEstimate.Nonce.Add(transactor_forGasEstimate.Nonce, big.NewInt(1))
newNonce, err := t.getNonce(ctx, chainID, address)
if err != nil {
return nil, fmt.Errorf("could not sign tx: %w", err)
}

Check warning on line 439 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L428-L439

Added lines #L428 - L439 were not covered by tests

// tx_forGasEstimate, err := call(transactor_forGasEstimate)
// if err != nil {
// return 0, fmt.Errorf("err contract call for gas est: %w", err)
// }
txType := t.txTypeForChain(chainID)

// fmt.Printf("tx_forGasEstimate: %v\n", tx_forGasEstimate.Gas())
transaction, err = util.CopyTX(transaction, util.WithNonce(newNonce), util.WithTxType(txType))
if err != nil {
return nil, fmt.Errorf("could not copy tx: %w", err)
}

Check warning on line 446 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L441-L446

Added lines #L441 - L446 were not covered by tests

//transactor.GasLimit = tx_forGasEstimate.Gas() + 555
//nolint: wrapcheck
return parentTransactor.Signer(address, transaction)

Check warning on line 449 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L449

Added line #L449 was not covered by tests
}

tx_forGasEstimate, err := call(transactor_forGasEstimate)

Check failure on line 452 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

var-naming: don't use underscores in Go names; var tx_forGasEstimate should be txForGasEstimate (revive)
if err != nil {
// at the moment, omniRPC gives a massive HTML doc w/ many sim errors.. reduce the noise.
errMsg := err.Error()
if strings.Contains(errMsg, "<!DOCTYPE html>") {
errMsg = strings.Split(errMsg, "<!DOCTYPE html>")[0] + "<html portion of error removed>"
}

Check warning on line 458 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L452-L458

Added lines #L452 - L458 were not covered by tests

// if arbitrum, spoof some low gas units to induce bump tests
if chainID.Uint64() == 10 {
transactor.GasLimit = 0
} else if chainID.Uint64() == 42161 {
transactor.GasLimit = 200000
return 0, fmt.Errorf("err contract call for gas est: %s", errMsg)

Check warning on line 460 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L460

Added line #L460 was not covered by tests
}
}

//tmpdebug
fmt.Printf("transactor.GasLimit: %d\n", transactor.GasLimit)
// with our gas limit now obtained from the simulation, apply this limit (plus any configured % modifier) to the
// gas limit of the actual transactor that is about to prepare the real transaction
gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainID.Uint64()))

Check failure on line 465 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)
transactor.GasLimit = tx_forGasEstimate.Gas() + (tx_forGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100)

Check failure on line 466 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion int -> uint64 (gosec)

Check warning on line 466 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L465-L466

Added lines #L465 - L466 were not covered by tests
}

var cancel context.CancelFunc
transactor.Context, cancel = context.WithTimeout(ctx, time.Second*5)
defer func() {
cancel()
}()
tx, err := call(transactor)
if err != nil {
return 0, fmt.Errorf("err contract call for tx: %w", err)

Check warning on line 471 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L471

Added line #L471 was not covered by tests
}

//tmpdebug
fmt.Printf("tx.Gas: %d\n", tx.Gas())

defer locker.Unlock()

//tmpdebug
fmt.Printf("SubmitTransaction>storeTX\n")

// now that we've stored the tx
err = t.storeTX(ctx, tx, db.Stored, uuid.New().String())
if err != nil {
Expand All @@ -489,9 +482,6 @@ func (t *txSubmitterImpl) SubmitTransaction(parentCtx context.Context, chainID *
span.AddEvent("trigger reprocess")
t.triggerProcessQueue(ctx)

//tmpdebug
fmt.Printf("SubmitTransaction>tx.Nonce: %d\n", tx.Nonce())

return tx.Nonce(), nil
}

Expand Down Expand Up @@ -732,31 +722,23 @@ func (t *txSubmitterImpl) getGasBlock(ctx context.Context, chainClient client.EV

// getGasEstimate gets the gas estimate for the given transaction.
// TODO: handle l2s w/ custom gas pricing through contracts.
func (t *txSubmitterImpl) getGasEstimate(ctx context.Context, chainClient client.EVM, chainID int, tx *types.Transaction) (gasLimit_new uint64, err error) {
func (t *txSubmitterImpl) getGasEstimate(ctx context.Context, chainClient client.EVM, chainID int, tx *types.Transaction) (gasLimit uint64, err error) {

// if dynamic gas estimation is not enabled, use cfg var gas_estimate as a default

Check warning on line 727 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L725-L727

Added lines #L725 - L727 were not covered by tests
if !t.config.GetDynamicGasEstimate(chainID) {
return t.config.GetGasEstimate(chainID), nil
}

//tmpdebug
fmt.Println("getGasEstimate>start")

gasUnitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(chainID)

Check warning on line 733 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L732-L733

Added lines #L732 - L733 were not covered by tests
//tmpdebug
fmt.Println("getGasEstimate>gasUnitAddPercentage", gasUnitAddPercentage)

ctx, span := t.metrics.Tracer().Start(ctx, "submitter.getGasEstimate", trace.WithAttributes(
attribute.Int(metrics.ChainID, chainID),

attribute.String(metrics.TxHash, tx.Hash().String()),

attribute.Int("gasUnitAddPercentage", gasUnitAddPercentage),

Check warning on line 737 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L737

Added line #L737 was not covered by tests
))

defer func() {
span.AddEvent("estimated_gas", trace.WithAttributes(attribute.Int64("gas", int64(gasLimit_new))))
span.AddEvent("estimated_gas", trace.WithAttributes(attribute.Int64("gas", int64(gasLimit))))

Check failure on line 741 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int64 (gosec)

Check warning on line 741 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L741

Added line #L741 was not covered by tests
metrics.EndSpanWithErr(span, err)
}()

Expand All @@ -766,34 +748,20 @@ func (t *txSubmitterImpl) getGasEstimate(ctx context.Context, chainClient client
return 0, fmt.Errorf("could not convert tx to call: %w", err)
}

// bump gas limit from prior by X%
gasLimit_fromPrior := tx.Gas()

gasLimit_fromEstimate, err := chainClient.EstimateGas(ctx, *call)

Check failure on line 751 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

var-naming: don't use underscores in Go names; var gasLimit_fromEstimate should be gasLimitFromEstimate (revive)

Check warning on line 752 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L751-L752

Added lines #L751 - L752 were not covered by tests
if err != nil {

//tmpdebug
fmt.Printf("getGasEstimate> Error estimating gas: %v\n", err)

span.AddEvent("could not estimate gas", trace.WithAttributes(attribute.String("error", err.Error())))

// if we failed to est gas for any reason, use *at least* the default flat gas on the next bump
gasLimit_fromEstimate = max(t.config.GetGasEstimate(chainID), gasLimit_fromEstimate)
// if we failed to est gas for any reason, use the default flat gas from config
gasLimit = t.config.GetGasEstimate(chainID)
} else {
// multiply the freshly simulated gasLimit by the configured gas unit add percentage
gasLimit_fromEstimate += (gasLimit_fromEstimate * uint64(gasUnitAddPercentage) / 100)

Check failure on line 760 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion int -> uint64 (gosec)
gasLimit = gasLimit_fromEstimate

Check warning on line 761 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L756-L761

Added lines #L756 - L761 were not covered by tests
}

// start with whichever value is higher gas from the prior attempt, or gas from our latest simulation estimate
gasLimit_new = max(gasLimit_fromPrior, gasLimit_fromEstimate)

// whichever source is used as the base, multiply it by the configured gas unit add percentage
gasLimit_new = gasLimit_new + (gasLimit_new * uint64(gasUnitAddPercentage) / 100)
span.AddEvent("new gas limit", trace.WithAttributes(
attribute.Int64("gas_limit", int64(gasLimit_new)),
attribute.Int64("gas_limit_from_prior", int64(gasLimit_fromPrior)),
attribute.Int64("gas_limit_from_estimate", int64(gasLimit_fromEstimate)),
))

return gasLimit_new, nil
return gasLimit, nil

Check warning on line 764 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L764

Added line #L764 was not covered by tests
}

func (t *txSubmitterImpl) Address() common.Address {
Expand Down
3 changes: 3 additions & 0 deletions services/rfq/contracts/fastbridgev2/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ var (
BridgeDepositClaimedTopic common.Hash
// BridgeProofDisputedTopic is the topic emitted by a bridge dispute.
BridgeProofDisputedTopic common.Hash
// BridgeProofDisputedTopic is a secondary topic emitted by a bridge request.
BridgeQuoteDetailsTopic common.Hash
)

// static checks to make sure topics actually exist.
Expand Down Expand Up @@ -67,6 +69,7 @@ func topicMap() map[EventType]common.Hash {
BridgeProofProvidedEvent: BridgeProofProvidedTopic,
BridgeDepositClaimedEvent: BridgeDepositClaimedTopic,
BridgeDisputeEvent: BridgeProofDisputedTopic,
BridgeQuoteDetailsEvent: BridgeQuoteDetailsTopic,
}
}

Expand Down
5 changes: 3 additions & 2 deletions services/rfq/contracts/fastbridgev2/eventtype_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions services/rfq/contracts/fastbridgev2/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ const (
BridgeDepositClaimedEvent
// BridgeDisputeEvent is the event type for the BridgeDispute event.
BridgeDisputeEvent
// BridgeQuoteDetailsEvent is emitted along w/ BridgeRequestedEvent as supplemental data

Check failure on line 26 in services/rfq/contracts/fastbridgev2/parser.go

View workflow job for this annotation

GitHub Actions / Lint (services/rfq)

Comment should end in a period (godot)
BridgeQuoteDetailsEvent
)

// Parser parses events from the fastbridge contracat.
Expand Down Expand Up @@ -91,6 +93,12 @@ func (p parserImpl) ParseEvent(log ethTypes.Log) (_ EventType, event interface{}
return noOpEvent, nil, false
}
return eventType, disputed, true
case BridgeQuoteDetailsEvent:
quoteDetails, err := p.filterer.ParseBridgeQuoteDetails(log)
if err != nil {
return noOpEvent, nil, false
}
return eventType, quoteDetails, true
}

return eventType, nil, true
Expand Down
20 changes: 0 additions & 20 deletions services/rfq/relayer/chain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ func (c Chain) SubmitRelay(ctx context.Context, request reldb.QuoteRequest) (uin
gasAmount := big.NewInt(0)
var err error

//tmpdebug
fmt.Println("SubmitRelay>start: ", request.OriginTxHash)

// Check to see if ETH should be sent to destination
if util.IsGasToken(request.Transaction.DestToken) {
gasAmount = request.Transaction.DestAmount
Expand All @@ -89,37 +86,20 @@ func (c Chain) SubmitRelay(ctx context.Context, request reldb.QuoteRequest) (uin
}
}

//tmpdebug
fmt.Println("SubmitRelay>SubmitTransaction: ", request.OriginTxHash)

nonce, err := c.SubmitTransaction(ctx, func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) {
transactor.Value = core.CopyBigInt(gasAmount)

//tmpdebug
callType := "exec"
if transactor.GasLimit == 0 {
callType = "sim"
}

//tmpdebug
fmt.Println(callType, "SubmitTransaction>RelayV2: ", request.OriginTxHash)

tx, err = c.Bridge.RelayV2(transactor, request.RawRequest, c.submitter.Address())

if err != nil {
return nil, fmt.Errorf("could not relay: %w", err)
}

//tmpdebug
fmt.Println(callType, "RelayV2 Return tx hash:", request.OriginTxHash, tx.Hash())

return tx, nil
})
if err != nil {
return 0, nil, fmt.Errorf("could not submit transaction: %w", err)
}
//tmpdebug
fmt.Println("SubmitRelay nonce:", nonce, "gas amount:", gasAmount)

return nonce, gasAmount, nil
}
10 changes: 9 additions & 1 deletion services/rfq/relayer/pricer/fee_pricer.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,17 @@ func (f *feePricer) getZapGasEstimate(ctx context.Context, destination uint32, q
callMsg.Value = quoteRequest.Transaction.ZapNative
}

// note: this gas amount is intentionally not modified
gasEstimate, err = client.EstimateGas(ctx, callMsg)
if err != nil {

Check failure on line 266 in services/rfq/relayer/pricer/fee_pricer.go

View workflow job for this annotation

GitHub Actions / Lint (services/rfq)

unnecessary leading newline (whitespace)
return 0, fmt.Errorf("could not estimate gas: %w", err)

// at the moment, omniRPC gives a massive HTML doc w/ many sim errors.. reduce the noise.
errMsg := err.Error()
if strings.Contains(errMsg, "<!DOCTYPE html>") {
errMsg = strings.Split(errMsg, "<!DOCTYPE html>")[0] + "<html portion of error removed>"
}

return 0, fmt.Errorf("could not estimate gas: %s", errMsg)
}

return gasEstimate, nil
Expand Down
11 changes: 1 addition & 10 deletions services/rfq/relayer/service/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,6 @@ func (q *QuoteRequestHandler) handleCommitPending(ctx context.Context, span trac
// TODO: just to be safe, we should probably check if another relayer has already relayed this.
func (q *QuoteRequestHandler) handleCommitConfirmed(ctx context.Context, span trace.Span, request reldb.QuoteRequest) (err error) {

//tmpdebug
fmt.Println("handleCommitConfirmed>SubmitRelay: ", request.OriginTxHash)

// TODO: store the dest txhash connected to the nonce
nonce, _, err := q.Dest.SubmitRelay(ctx, request)
if err != nil {
Expand All @@ -388,15 +385,11 @@ func (q *QuoteRequestHandler) handleCommitConfirmed(ctx context.Context, span tr
span.AddEvent("relay successfully submitted")
span.SetAttributes(attribute.Int("relay_nonce", int(nonce)))

//tmpdebug
fmt.Println("handleCommitConfirmed>UpdateQuoteRequestStatus: ", request.OriginTxHash)
err = q.db.UpdateQuoteRequestStatus(ctx, request.TransactionID, reldb.RelayStarted, &request.Status)
if err != nil {
return fmt.Errorf("could not update quote request status: %w", err)
}

//tmpdebug
fmt.Println("handleCommitConfirmed>UpdateRelayNonce: ", request.OriginTxHash)
err = q.db.UpdateRelayNonce(ctx, request.TransactionID, nonce)
if err != nil {
return fmt.Errorf("could not update relay nonce: %w", err)
Expand All @@ -420,7 +413,7 @@ func (r *Relayer) handleRelayLog(parentCtx context.Context, req *fastbridgev2.Fa

reqID, err := r.db.GetQuoteRequestByID(ctx, req.TransactionId)
if err != nil {
return fmt.Errorf("could not get quote request: %w", err)
return fmt.Errorf("could not get quote request for tx ID %s: %w", hexutil.Encode(req.TransactionId[:]), err)
}
// we might've accidentally gotten this later, if so we'll just ignore it
// note that in the edge case where we pessimistically marked as DeadlineExceeded
Expand Down Expand Up @@ -481,13 +474,11 @@ func (q *QuoteRequestHandler) handleRelayCompleted(ctx context.Context, span tra
// relay has been finalized, it's time to go back to the origin chain and try to prove
_, err = q.Origin.SubmitTransaction(ctx, func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) {

fmt.Println("SubmitTransaction>Prove ", request.OriginTxHash, "Nonce:", transactor.Nonce)
tx, err = q.Origin.Bridge.Prove(transactor, request.RawRequest, request.DestTxHash)
if err != nil {
return nil, fmt.Errorf("could not prove: %w", err)
}

fmt.Println("SubmitTransaction>Prove return tx.gas ", tx.Gas())
return tx, nil
})
if err != nil {
Expand Down

0 comments on commit 73442fa

Please sign in to comment.