Skip to content

Develop -> Master#2025

Merged
tynes merged 149 commits intomasterfrom
develop
Jan 18, 2022
Merged

Develop -> Master#2025
tynes merged 149 commits intomasterfrom
develop

Conversation

@optimisticben
Copy link
Contributor

Description
Develop -> Master

barrasso and others added 30 commits December 9, 2021 16:11
This commit places the batch sequencer tx size check inside of a for
loop so that the we can continue to whittle away batch tx sizes until
satisfying the configured maximum. This doesn't result in a behavioral
change, as the for loop exits after the first iteration. This is done to
make the behavioral changes more apparent in subsequent commits.
..by breaking its require(ments)
… example output.

This is a WIP commit with the purpose of quick feedback on the result.
…e are any missing Natspec comments.

Still WIP. Better console formatting + configurations is next :)
- within fraud proof window
- not batchheader timestamp equal zero
Create a setup script for the integration tests
feat: implement getMessageReceipt and tests
smartcontracts and others added 16 commits January 14, 2022 13:29
feat: implement clear pending txs for go batch submitter
Fixes a bug where both the sequencer and proposer main loops were
attempting to clear the pending transactions from the batch-tx
submitter's wallet. The impact is that we may have been overspending on
fees due to conflicting/reverting state batches. However the impact
overall should be minor given the relative size of state batches in
comparison to tx-batches.
fix(batch-submitter): clear state root batches
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.
bss: split up SubmitBatchTx method in Driver iface
We need to manually include musl headers and GCC now that proxyd has a geth dependency and needs CGO.
fixed file extensions in integration tests
@changeset-bot
Copy link

changeset-bot bot commented Jan 18, 2022

🦋 Changeset detected

Latest commit: 3174f03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@eth-optimism/integration-tests Patch
@eth-optimism/proxyd Minor
@eth-optimism/data-transport-layer Patch
@eth-optimism/op-exporter Patch
@eth-optimism/gas-oracle Patch
@eth-optimism/contracts Patch
@eth-optimism/l2geth Patch
@eth-optimism/message-relayer Patch
@eth-optimism/batch-submitter Patch
@eth-optimism/sdk Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions github-actions bot added 2-reviewers A-op-batcher Area: op-batcher M-ci Meta: ci related work A-pkg-core-utils Area: packages/core-utils A-integration Area: integration tests A-cannon Area: cannon A-ops Area: ops labels Jan 18, 2022
@tynes tynes merged commit 606873d into master Jan 18, 2022
theochap added a commit that referenced this pull request Dec 10, 2025
…s to a given peer (#2025)

## Description

This PR fixes the gossip stability issues we've experience on initial
connections to the `op-node`s by preventing `kona-node`s from redialing
already connected peers, or peers that have been already dialed.

With that fix on, we're not experiencing the peer drops in kurtosis
anymore

On a side note: this PR also increases the log levels of some gossip
events.

## Explanation

My understanding of the situation:

- The libp2p library imposes a substream limit for outgoing connections
(constant, equal to 5). This is to prevent DOS vectors when creating new
connections
- It seems that every dial call will open a new outbound substream.
- Since we're dialing peers quite often in the discovery layer, we're
causing the libp2p library to reach the substream limit
- When we reach the substream limit, the protocol gets disconnected and
we're not advertising it to peers anymore. This causes the gossip
connection to drop

Relevant piece of code in libp2p :
https://github.com/libp2p/rust-libp2p/blob/d3e88cfc2ec944c3e6beb7117a762452cb855e38/protocols/gossipsub/src/handler.rs#L499-L511.
In our case, the outbound event is consistently the
ConnectionEvent::FullyNegotiatedOutbound event, which happens on every
new dial.

## Development

- Close #1854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cannon Area: cannon A-integration Area: integration tests A-op-batcher Area: op-batcher A-ops Area: ops A-pkg-core-utils Area: packages/core-utils M-ci Meta: ci related work

Projects

None yet

Development

Successfully merging this pull request may close these issues.