Skip to content

bss: split up SubmitBatchTx method in Driver iface#2008

Merged
mslipper merged 3 commits intoethereum-optimism:developfrom
cfromknecht:bss-craft-then-send-tx
Jan 14, 2022
Merged

bss: split up SubmitBatchTx method in Driver iface#2008
mslipper merged 3 commits intoethereum-optimism:developfrom
cfromknecht:bss-craft-then-send-tx

Conversation

@cfromknecht
Copy link
Contributor

@cfromknecht cfromknecht commented Jan 14, 2022

Description
The current Driver interface supports only one method for both crafting and
publishing a batch transaction, SubmitBatchTx. Currently this is being
executed on each new gas price that the txmgr commands, implying we are
doing a lot of extra work to rederive batches. In addition, much our
instrumentation lives inside this method, meaning that the they are also
being recorded multiple times per transaction. When we get to
processing larger batches on Kovan and Mainnet, this could also become a
resource bottleneck.

This PR remedies all of the above issues by splitting out the
transaction crafting process from the publication. A new method,
CraftBatchTx, is added to the Driver interface. This method is
responsible for creating a fully formed transaction, but does not
publish it. The responsibility of SubmitBatchTx is now modified to take
an predefined transaction, i.e. created by CraftBatchTx, overwrite the
supplied gas price, and publish. In this way, this expensive call to
build batches can be done once before handing the transaction of the
txmgr.

Additionally, we now log the raw byte length for the batch_tx_size metric
rather than tx.Size(). The latter returns a float value of type StorageSize,
which explains why the metrics haven't been indicating the actual byte values.

These commits are carried over from prior work done on #1993.

Builds on

Metadata

  • Fixes ENG-1891

@changeset-bot
Copy link

changeset-bot bot commented Jan 14, 2022

⚠️ No Changeset found

Latest commit: 961dfe2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2022

Codecov Report

Merging #2008 (961dfe2) into develop (e5ba9b1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2008   +/-   ##
========================================
  Coverage    74.58%   74.58%           
========================================
  Files           79       79           
  Lines         2554     2554           
  Branches       401      401           
========================================
  Hits          1905     1905           
  Misses         649      649           
Flag Coverage Δ
batch-submitter 62.50% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.64% <ø> (ø)
message-relayer 70.86% <ø> (ø)
sdk 88.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5ba9b1...961dfe2. Read the comment docs.

@cfromknecht cfromknecht force-pushed the bss-craft-then-send-tx branch 2 times, most recently from a96bc0b to ae414aa Compare January 14, 2022 19:17
@cfromknecht cfromknecht force-pushed the bss-craft-then-send-tx branch from ae414aa to a051271 Compare January 14, 2022 19:49
The current interface supports only one method for both crafting and
publishing a batch transaction, SubmitBatchTx. Currently this is being
executed on each new gas price that the txmgr commands, implying we are
doing a lot of extra work to rederive batches. In addition, much our
instrumentation lives inside this method, meaning that the they are also
being recorded multiple times per transaction. When we get to
processing larger batches on Kovan and Mainnet, this could also become a
resource bottleneck.

This commit remedies all of the above issues by splitting out the
transaction crafting process from the publication. A new method,
CraftBatchTx, is added to the Driver interface. This method is
responsible for creating a fully formed transaction, but does not
publish it. The responsibility of SubmitBatchTx is now modified to take
an predefined transaction, i.e. created by CraftBatchTx, overwrite the
supplied gas price, and publish. In this way, this expensive call to
build batches can be done once before handing the transaction of the
txmgr.
Previously it was being logged every time we modified the gas price.
Additionally, we log the raw byte length rather than tx.Size(). The
latter returns a float value of type StorageSize, which explains why the
metrics haven't been indicating actual byte values.
@cfromknecht cfromknecht force-pushed the bss-craft-then-send-tx branch from a051271 to 961dfe2 Compare January 14, 2022 20:18
@mslipper mslipper merged commit 235c7af into ethereum-optimism:develop Jan 14, 2022
@cfromknecht cfromknecht deleted the bss-craft-then-send-tx branch January 14, 2022 21:34
theochap pushed a commit that referenced this pull request Dec 10, 2025
### Description

Adds metrics around frames and channels in the pipeline stages.

Closes #1985
Closes #1992

---------

Co-authored-by: clabby <ben@clab.by>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants