-
Notifications
You must be signed in to change notification settings - Fork 450
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
Implement Express Lane Timeboost #2561
base: master
Are you sure you want to change the base?
Conversation
Updated auctioneer with research spec
…o into express-lane-timeboost
initialTimestamp := time.Unix(int64(roundTimingInfo.OffsetTimestamp), 0) | ||
roundDuration := time.Duration(roundTimingInfo.RoundDurationSeconds) * time.Second | ||
auctionClosingDuration := time.Duration(roundTimingInfo.AuctionClosingSeconds) * time.Second |
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 can add an assertion here that the roundTimingInfo complies with the assumptions we've got baked into the code currently (eg duration == 1 minute, offset is a time at a minute boundary, closing duration is at least 2s but probably just assert it's 15s)
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.
roundTimingInfo
is retrieved from the smart contract, which is the source of truth we should adhere to. Unless we want to add another configuration to check against this single source of truth, I generally prefer less configuration, as it's less error-prone, but I'm open to changing if that's what we prefer!
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 that we should rely on the contract here as the source of truth
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.
My point is that the code is broken if it's anything other than 1 minute. The contract is the source of truth so we should check that the settings on the contract matches the assumptions we're currently making in the code, and assert if they are violated because otherwise the code will behave in unexpected ways.
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.
This issue is now fixed by #2775
Can resolve this thread once it's merged back in.
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
redisURL := redisutil.CreateTestRedis(ctx, t) | ||
_ = redisURL |
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.
This test needs to be filled out.
autonomous-auctioneer on the cli was failing to start becuase we were adding the "auth" config options without having the corresponding field on the AuctioneerConfig. We can add it back in later if needed.
RPC methods can't be registered after the stack is started.
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.
initial review of code inside gethexec
@@ -430,6 +481,12 @@ func (s *Sequencer) PublishTransaction(parentCtx context.Context, tx *types.Tran | |||
return err | |||
} | |||
|
|||
if s.config().Timeboost.Enable && s.expressLaneService != nil { | |||
if delay && s.expressLaneService.currentRoundHasController() { |
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.
that tells me "delay" is a misleading name - because it's actually delay only if current round has controller.
Maybe an inverted "fastLane" boolean, or something else?
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.
Fix included in #2787
if err := s.expressLaneService.validateExpressLaneTx(msg); err != nil { | ||
return err | ||
} | ||
return s.expressLaneService.sequenceExpressLaneSubmission(ctx, msg, s.publishTransactionImpl) |
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 really don't like passing member functions, especially private ones, as function pointers if there is a reasonable alternative. Can't the expressLaneService get a pointer to sequencer instead?
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.
Fix included in #2787
return err | ||
} | ||
// Increase the global round sequence number. | ||
control.sequence += 1 |
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.
so if publishTxnFn fails, control never advances and anything on the timeboost queue is waiting until publisher notices that and replaces the message with that sequence number?
I'm not sure this is how we want to go. Will be problematic, especially if the winner allows transactions from multiple sources.
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.
The sequence number mechanism for express lane submissions works analogously to Ethereum account nonces:
-
Just as multiple entities sharing an Ethereum account must coordinate their nonce usage, multiple entities sharing an express lane controller address must coordinate sequence numbers.
-
In both cases:
- If a transaction fails validation before reaching the mempool (e.g., invalid nonce), the account nonce is not incremented
- If a transaction is included in a block, the nonce increments regardless of execution success/revert
For express lane submissions, there are two layers of validation:
- The outer
ExpressLaneSubmission
envelope (sequence number, signatures, etc.) - The inner transaction validation (nonces, gas, etc.)
The sequence number only increments if both layers of validation pass and the transaction is queued for inclusion in a block. This matches Ethereum's behavior where nonces only increment for transactions that make it into blocks.
The errors returned by timeboost_sendExpressLaneTransaction
are about validation and inclusion, not execution results - just like Ethereum's mempool validation. This makes the current sequence number behavior consistent with established patterns for handling transaction ordering.
return timeboost.ErrNoOnchainController | ||
} | ||
// Check if the submission nonce is too low. | ||
if msg.Sequence < control.sequence { |
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.
msg.Sequence is mandatory?
Do't we want to allow sending e.g. with sequence number 0 to be processed FCFS?
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.
The sequence number is a mandatory part of the express lane protocol design. Its purpose is to give express lane controllers explicit control over transaction ordering within their round, similar to how Ethereum account nonces work. Processing express lane submissions FCFS would defeat this purpose, as network conditions could reorder transactions against the controller's intent. The spec requires each submission to increment the sequence number by one, with sequence 0 being the starting point for each new round or when controller rights are transferred. This ensures deterministic ordering control for the party that won the auction rights.
} | ||
// Log an informational warning if the message's sequence number is in the future. | ||
if msg.Sequence > control.sequence { | ||
log.Warn("Received express lane submission with future sequence number", "sequence", msg.Sequence) |
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.
Warn seems too harsh. This will happen regularly due to network reordering.
Log.Info or less
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.
Fix included in #2787
} | ||
} | ||
}) | ||
es.LaunchThread(func(ctx context.Context) { |
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.
use CCallIteratively to do something every 250mil or until context is cancelled, and move logic to a separate function
"round", it.Event.Round, | ||
"controller", it.Event.FirstPriceExpressLaneController, | ||
) | ||
es.Lock() |
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 think locking deserves some overview.
Would suggest trying to split between a lock used while processing incoming transaction in sequenceExpressLaneSubmission (which writes the message sequence and only reads round info of current round) vs lock used while while processing new round info (which doesn't care about incoming transactions and writes - hopefully only next-round). If needed, incoming-Tx may hold biefly the round lock just to check current round info
return timeboost.ErrNoOnchainController | ||
} | ||
currentRound := timeboost.CurrentRound(es.initialTimestamp, es.roundDuration) | ||
if msg.Round != currentRound { |
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 think we would want some buffering for next-round messages, to make sure winner can use their entire slot.
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 thought of a straight forward way to address this: #2788
if !s.config().Timeboost.Enable { | ||
log.Crit("Timeboost is not enabled, but StartExpressLane was called") | ||
} | ||
rpcClient, err := rpc.DialContext(ctx, s.config().Timeboost.SequencerHTTPEndpoint) |
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 really don't like this.
expresslaneservice is inside execution and has direct access to the blockchain, no reason to go through network.
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.
Addressed in #2762
If the sequencer restarts just after the ExpressLaneAuction contract is deployed, it may not be fully synced up to that point when it starts up again. This commit adds in retries with exponential backoff up to 4 seconds.
[NIT-2876] Fix panic when transferring express lane controller due to race in lru cache
…mber-attr Fix SequenceNumber attribute and decode order
[NIT-2876] Fix typo causing panic
Background
At the time of writing, the Arbitrum sequencer is centralized and offers a first-come, first-serve transaction ordering policy. Txs have a current delay of approximately 250ms, which is the time the sequencer takes to produce an ordered list of txs to emit in the form of an L2 block. The current policy does not handle MEV that occurs naturally on L2, and leads to latency races offline to get faster access to the sequencer ingress server.
A new policy has been proposed, known as Express Lane Timeboost, which allows participants to bid for the rights of priority sequencing using their funds instead of hardware. In “rounds” that start at each minute mark, participants can submits bids to participate in a sealed, second-price auction for control of the next round’s “express lane”. During a round, all non-express lane txs get their first arrival timestamp delayed by some amount of time (250ms), while the express lane controller does not. The express lane controller can also choose to transfer their rights in a round.
The sequencer itself does not need to manage auctions, but simply needs to know the current round number and the address of the express lane controller for that round. From there, it can delay non-express lane txs by a nominal amount required by the protocol and validate that a tx should go through the express lane.
This PR contains the complete implementation of the system with all its components. The smart contract changes are contained within OffchainLabs/nitro-contracts/tree/express-lane-auction-all-merged.
Basic Readings
To read more about timeboost, see the AIP, the research specification, and design doc although the design doc is not fully updated yet.
Reviewing
Recommend to look at the basic readings, then look at
system_tests/timeboost_test.go
to understand how it all fits together. Then, look at bid validator and auctioneer. Finally, the sequencer changes.Features
Sequencer Changes
The changes to the sequencer hot path are quite simple. In a nutshell, if a transaction is received, it checks the following:
If timeboost is enabled AND there is an express lane controller set AND it is not coming from the express lane, it delays the tx's first arrival timestamp by some amount (250ms).
To determine if a transaction is a valid express lane tx, the sequencer runs a background thread called the
expressLaneService
, which is scraping events from the ExpressLaneAuction.sol smart contract. Express lane transactions arrive via a different sequencer endpoint than the normal one, calledtimeboost_sendExpressLaneTransaction
. The message looks as follows:The submission itself contains a tx payload, which MAY not be from the express lane controller. As long as the submission is signed by the controller, that is sufficient. Submissions have a specific nonce, called a sequence, to ensure that submissions are processed in order. This is different from the inner nonce of the payload tx. The sequencer keeps a queue of submissions and ensures it processes them in order. That is, if a submission N is received before N-1, it will get queued for submission once N arrives.
Bid Validator Architecture
Bids are limited to 5 bids per sender, but there are no limits to the number of bidders in a single round. To alleviate potential scaling concerns, we adopt a simple architecture of separating the bid validators from the auctioneer. The bid validators filter out invalid items and publish validated results to a Redis stream. In a simplified diagram, here's what it will look like:
Dependencies Added
Notes
There are several parts of this implementation that are likely not ideal:
Chicken and the egg problem in sequencer
Cannot start sequencer without express lane, but cannot deploy auction for express lane without starting sequencer. To solve this in tests, we have a separate func called
StartExpressLaneService
in the sequencer. In prod, we don’t have this issue because we can deploy the contracts before we upgrade the sequencer to timeboost, but what to do about tests?Janky prioritizing of auction resolution txs
The sequencer exposes an authenticated endpoint
auctioneer_submitAuctionResolutionTransaction
over the JWT Auth RPC for the auctioneer to use. When the auctioneer is ready to resolve an auction, it submits a tx to this endpoint, which the sequencer verifies for integrity. Then, the sequencer does the following:it immediately tries to put the item in the queue and create block. It also sets the tx as a property of the sequencer struct, and in the
createBlock
func, if this field is not nil, it gets put at the top of the queue. This is a bit janky in how it works and perhaps inefficient. Is there another way to prioritize a tx in the sequencer?Sequencer opens an http connection to itself
The sequencer has a thread called
expressLaneService
which reads events from the auction smart contracts on L2 to determine express lane controllers. Because the sequencer does not havefiltersystem
API access, we instead open an RPC client against itself so we can create anethclient
to read logs and data from onchain. This doesn't seem idealReferences