-
Notifications
You must be signed in to change notification settings - Fork 12
Update tx submission logic to only fetch a single account key from the COA #911
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
Changes from all commits
f3f95f9
224a2ba
f11053e
caf7c14
4bbbf75
253d859
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 |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ import ( | |
| flowGo "github.com/onflow/flow-go/model/flow" | ||
| "github.com/rs/zerolog" | ||
| "github.com/sethvargo/go-retry" | ||
| "golang.org/x/sync/errgroup" | ||
|
|
||
| "github.com/onflow/flow-evm-gateway/config" | ||
| "github.com/onflow/flow-evm-gateway/metrics" | ||
|
|
@@ -93,15 +92,15 @@ func (t *SingleTxPool) Add( | |
| return err | ||
| } | ||
|
|
||
| latestBlock, account, err := t.fetchFlowLatestBlockAndCOA(ctx) | ||
| latestBlock, err := t.client.GetLatestBlock(ctx, true) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| script := replaceAddresses(runTxScript, t.config.FlowNetworkID) | ||
| flowTx, err := t.buildTransaction( | ||
| ctx, | ||
| latestBlock, | ||
| account, | ||
| script, | ||
| cadence.NewArray([]cadence.Value{hexEncodedTx}), | ||
| coinbaseAddress, | ||
|
|
@@ -157,8 +156,8 @@ func (t *SingleTxPool) Add( | |
| // buildTransaction creates a Cadence transaction from the provided script, | ||
| // with the given arguments and signs it with the configured COA account. | ||
| func (t *SingleTxPool) buildTransaction( | ||
| ctx context.Context, | ||
| latestBlock *flow.Block, | ||
| account *flow.Account, | ||
| script []byte, | ||
| args ...cadence.Value, | ||
| ) (*flow.Transaction, error) { | ||
|
|
@@ -177,53 +176,35 @@ func (t *SingleTxPool) buildTransaction( | |
| } | ||
| } | ||
|
|
||
| // building and signing transactions should be blocking, | ||
| // so we don't have keys conflict | ||
| t.mux.Lock() | ||
| defer t.mux.Unlock() | ||
| accKey, err := t.fetchSigningAccountKey() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| accKey, err := t.keystore.Take() | ||
| coaAddress := t.config.COAAddress | ||
| accountKey, err := t.client.GetAccountKeyAtLatestBlock(ctx, coaAddress, accKey.Index) | ||
| if err != nil { | ||
| accKey.Done() | ||
| return nil, err | ||
| } | ||
|
|
||
| if err := accKey.SetProposerPayerAndSign(flowTx, account); err != nil { | ||
| if err := accKey.SetProposerPayerAndSign(flowTx, coaAddress, accountKey); err != nil { | ||
| accKey.Done() | ||
| return nil, err | ||
| } | ||
|
|
||
| // now that the transaction is prepared, store the transaction's metadata | ||
| accKey.SetLockMetadata(flowTx.ID(), latestBlock.Height) | ||
|
|
||
| t.collector.OperatorBalance(account) | ||
|
Collaborator
Author
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 removed the |
||
|
|
||
| return flowTx, nil | ||
| } | ||
|
|
||
| func (t *SingleTxPool) fetchFlowLatestBlockAndCOA(ctx context.Context) ( | ||
| *flow.Block, | ||
| *flow.Account, | ||
| error, | ||
| ) { | ||
| var ( | ||
| g = errgroup.Group{} | ||
| err1, err2 error | ||
| latestBlock *flow.Block | ||
| account *flow.Account | ||
| ) | ||
|
|
||
| // execute concurrently so we can speed up all the information we need for tx | ||
| g.Go(func() error { | ||
| latestBlock, err1 = t.client.GetLatestBlock(ctx, true) | ||
| return err1 | ||
| }) | ||
| g.Go(func() error { | ||
| account, err2 = t.client.GetAccount(ctx, t.config.COAAddress) | ||
| return err2 | ||
| }) | ||
| if err := g.Wait(); err != nil { | ||
| return nil, nil, err | ||
| } | ||
| func (t *SingleTxPool) fetchSigningAccountKey() (*keystore.AccountKey, error) { | ||
| // getting an account key from the `KeyStore` for signing transactions, | ||
| // should be lock-protected, so that we don't sign any two Flow | ||
| // transactions with the same account key | ||
| t.mux.Lock() | ||
| defer t.mux.Unlock() | ||
|
|
||
| return latestBlock, account, nil | ||
| return t.keystore.Take() | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could throttle the calls to
GetLatestBlock, given the latest block only changes about once per second for mainnet, and we are only using the block ID as reference block ID, it's ok to have a little bit delay. This could further reduce the load to AN.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, there should be a component that is responsible for querying (or being subscribed to) the last block, which we then use here.
I think that might be best to put in a separate PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already opened a separate PR for that: #896