-
Notifications
You must be signed in to change notification settings - Fork 14
Oev 671 txm improvements #294
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.
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
976ba44
e533378
a822f17
91c5a2d
81ef8e6
64f4d2f
7ef98a2
c3d4502
115e871
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,8 +27,11 @@ import ( | |
| ) | ||
|
|
||
| const ( | ||
| timeout = time.Second * 5 | ||
| metaABI = `[ | ||
| timeout = time.Second * 5 | ||
| NoBidsError = "no bids" | ||
| NoSolverOps = "no solver operations received" | ||
| NoSolverOpsAfterSimulation = "no valid solver operations after simulation" | ||
| metaABI = `[ | ||
| { | ||
| "type": "function", | ||
| "name": "metacall", | ||
|
|
@@ -142,7 +145,7 @@ func NewMetaClient(lggr logger.Logger, c MetaClientRPC, ks MetaClientKeystore, c | |
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the MetaClient currently active -- it looks A-specific? Ideally, we want to ensure the client is decoupled so that a beholder outage doesn't block the main flow. For example, on 10/10 I saw many alerts about beholder potentially having issues. |
||
|
|
||
| return &MetaClient{ | ||
| lggr: logger.Sugared(logger.Named(lggr, "Txm.Txm.MetaClient")), | ||
| lggr: logger.Sugared(logger.Named(lggr, "Txm.MetaClient")), | ||
| c: c, | ||
| ks: ks, | ||
| customURL: customURL, | ||
|
|
@@ -179,7 +182,7 @@ func (a *MetaClient) SendTransaction(ctx context.Context, tx *types.Transaction, | |
| return nil | ||
| } | ||
| a.lggr.Infof("No bids for transactionID(%d): ", tx.ID) | ||
| return nil | ||
| return errors.New(NoBidsError) | ||
| } | ||
| a.lggr.Infow("Broadcasting attempt to public mempool", "tx", tx) | ||
| return a.c.SendTransaction(ctx, attempt.SignedTransaction) | ||
|
|
@@ -355,7 +358,7 @@ func (a *MetaClient) SendRequest(parentCtx context.Context, tx *types.Transactio | |
| } | ||
|
|
||
| if response.Error.ErrorMessage != "" { | ||
| if strings.Contains(response.Error.ErrorMessage, "no solver operations received") { | ||
| if strings.Contains(response.Error.ErrorMessage, NoSolverOps) || strings.Contains(response.Error.ErrorMessage, NoSolverOpsAfterSimulation) { | ||
| a.metrics.RecordBidsReceived(ctx, 0) | ||
| return nil, nil | ||
| } | ||
|
|
@@ -521,7 +524,7 @@ func (a *MetaClient) SendOperation(ctx context.Context, tx *types.Transaction, a | |
| if err != nil { | ||
| return fmt.Errorf("failed to sign attempt for txID: %v, err: %w", tx.ID, err) | ||
| } | ||
| a.lggr.Infow("Intercepted attempt for tx", "txID", tx.ID, "toAddress", meta.ToAddress, "gasLimit", meta.GasLimit, | ||
| a.lggr.Infow("Intercepted attempt for tx", "txID", tx.ID, "hash", signedTx.Hash(), "toAddress", meta.ToAddress, "gasLimit", meta.GasLimit, | ||
| "TipCap", tip, "FeeCap", meta.MaxFeePerGas) | ||
| return a.c.SendTransaction(ctx, signedTx) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||
| package dualbroadcast | ||||||
|
|
||||||
| import ( | ||||||
| "context" | ||||||
| "fmt" | ||||||
| "strings" | ||||||
|
|
||||||
| "github.com/ethereum/go-ethereum/common" | ||||||
|
|
||||||
| "github.com/smartcontractkit/chainlink-evm/pkg/txm" | ||||||
| "github.com/smartcontractkit/chainlink-evm/pkg/txm/types" | ||||||
| ) | ||||||
|
|
||||||
| type errorHandler struct{} | ||||||
|
|
||||||
| func NewErrorHandler() *errorHandler { | ||||||
| return &errorHandler{} | ||||||
| } | ||||||
|
|
||||||
| func (e *errorHandler) HandleError(ctx context.Context, tx *types.Transaction, txErr error, txStore txm.TxStore, setNonce func(common.Address, uint64), isFromBroadcastMethod bool) error { | ||||||
| // If this isn't the first broadcast, don't mark the tx as fatal as other txs might be included on-chain. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if strings.Contains(txErr.Error(), NoBidsError) && tx.AttemptCount == 1 { | ||||||
| if err := txStore.MarkTxFatal(ctx, tx, tx.FromAddress); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| setNonce(tx.FromAddress, *tx.Nonce) | ||||||
| return fmt.Errorf("transaction with txID: %d marked as fatal", tx.ID) | ||||||
| } | ||||||
|
|
||||||
| return txErr | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,12 +12,16 @@ import ( | |
| "github.com/smartcontractkit/chainlink-evm/pkg/txm" | ||
| ) | ||
|
|
||
| func SelectClient(lggr logger.Logger, client client.Client, keyStore keys.ChainStore, url *url.URL, chainID *big.Int) (txm.Client, error) { | ||
| func SelectClient(lggr logger.Logger, client client.Client, keyStore keys.ChainStore, url *url.URL, chainID *big.Int) (txm.Client, txm.ErrorHandler, error) { | ||
| urlString := url.String() | ||
| switch { | ||
| case strings.Contains(urlString, "flashbots"): | ||
| return NewFlashbotsClient(client, keyStore, url), nil | ||
| return NewFlashbotsClient(client, keyStore, url), nil, nil | ||
| default: | ||
| return NewMetaClient(lggr, client, keyStore, url, chainID) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code could then be made cleaner if |
||
| mc, err := NewMetaClient(lggr, client, keyStore, url, chainID) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| return mc, NewErrorHandler(), nil | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| package storage | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "math/big" | ||
| "sort" | ||
|
|
@@ -122,6 +121,7 @@ func (m *InMemoryStore) CreateEmptyUnconfirmedTransaction(nonce uint64, gasLimit | |
| SpecifiedGasLimit: gasLimit, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: We could simplify this by hardcoding the gas limit to 21_000. Since a standard ETH transfer to an EOA (0x0) has a fixed cost (21_000), passing in gasLimit and maintaining a separate EmptyTxLimitDefault config seems like unnecessary overhead. In addition, it would also remove the need to add If we were to instead send ETH to/from a smart contract, only then would the gas amount differ. |
||
| CreatedAt: time.Now(), | ||
| State: txmgr.TxUnconfirmed, | ||
| IsPurgeable: true, | ||
| } | ||
|
|
||
| if _, exists := m.UnconfirmedTransactions[nonce]; exists { | ||
|
|
@@ -366,8 +366,15 @@ func (m *InMemoryStore) DeleteAttemptForUnconfirmedTx(transactionNonce uint64, a | |
| return fmt.Errorf("attempt with hash: %v for txID: %v was not found", attempt.Hash, attempt.TxID) | ||
| } | ||
|
|
||
| func (m *InMemoryStore) MarkTxFatal(*types.Transaction) error { | ||
| return errors.New("not implemented") | ||
| func (m *InMemoryStore) MarkTxFatal(txToMark *types.Transaction) error { | ||
| m.Lock() | ||
| defer m.Unlock() | ||
|
|
||
| // TODO: for now do the simple thing and drop the transaction instead of adding it to the fatal queue. | ||
| delete(m.UnconfirmedTransactions, *txToMark.Nonce) | ||
| delete(m.Transactions, txToMark.ID) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a few safety concerns about deleting these entries:
|
||
| txToMark.State = txmgr.TxFatalError // update the state in case the caller needs to log | ||
| return nil | ||
| } | ||
|
|
||
| // Orchestrator | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.