Skip to content
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.

Sign up for GitHub

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

Add early timeboost submission grace period #2788

Merged
merged 9 commits into from
Dec 11, 2024
Merged

Add early timeboost submission grace period #2788

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a8a6369
Add early timeboost submission grace period
Tristan-Wilson Nov 13, 2024
5e450cd
Merge remote-tracking branch 'origin/expres-lane-timeboost-fix-pr-com…
Tristan-Wilson Nov 20, 2024
2bea5ca
Loop for round check
Tristan-Wilson Nov 20, 2024
fbbc94e
Merge branch 'express-lane-timeboost' into express-lane-timeboost-ear…
Tristan-Wilson Nov 21, 2024
bcabeaa
Fix race condition around checking current round
Tristan-Wilson Nov 21, 2024
06ea35b
Merge branch 'express-lane-timeboost' into express-lane-timeboost-ear…
Tristan-Wilson Dec 9, 2024
8175908
Merge branch 'express-lane-timeboost' into express-lane-timeboost-ear…
Tristan-Wilson Dec 11, 2024
c2b85d2
Move currentTime into loop
Tristan-Wilson Dec 11, 2024
6b8fd67
Merge branch 'express-lane-timeboost' into express-lane-timeboost-ear…
Tristan-Wilson Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/nitro/nitro.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,9 @@ func mainImpl() int {
execNode.Sequencer.StartExpressLane(
ctx,
common.HexToAddress(execNodeConfig.Sequencer.Timeboost.AuctionContractAddress),
common.HexToAddress(execNodeConfig.Sequencer.Timeboost.AuctioneerAddress))
common.HexToAddress(execNodeConfig.Sequencer.Timeboost.AuctioneerAddress),
execNodeConfig.Sequencer.Timeboost.EarlySubmissionGrace,
)
}

err = nil
Expand Down
10 changes: 9 additions & 1 deletion execution/gethexec/express_lane_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type expressLaneService struct {
initialTimestamp time.Time
roundDuration time.Duration
auctionClosing time.Duration
earlySubmissionGrace time.Duration
chainConfig *params.ChainConfig
logs chan []*types.Log
seqClient *ethclient.Client
Expand All @@ -56,6 +57,7 @@ func newExpressLaneService(
auctionContractAddr common.Address,
sequencerClient *ethclient.Client,
bc *core.BlockChain,
earlySubmissionGrace time.Duration,
) (*expressLaneService, error) {
chainConfig := bc.Config()
auctionContract, err := express_lane_auctiongen.NewExpressLaneAuction(auctionContractAddr, sequencerClient)
Expand Down Expand Up @@ -90,6 +92,7 @@ pending:
chainConfig: chainConfig,
initialTimestamp: initialTimestamp,
auctionClosing: auctionClosingDuration,
earlySubmissionGrace: earlySubmissionGrace,
roundControl: lru.NewCache[uint64, *expressLaneControl](8), // Keep 8 rounds cached.
auctionContractAddr: auctionContractAddr,
roundDuration: roundDuration,
Expand Down Expand Up @@ -295,7 +298,12 @@ func (es *expressLaneService) validateExpressLaneTx(msg *timeboost.ExpressLaneSu
}
currentRound := timeboost.CurrentRound(es.initialTimestamp, es.roundDuration)
if msg.Round != currentRound {
return errors.Wrapf(timeboost.ErrBadRoundNumber, "express lane tx round %d does not match current round %d", msg.Round, currentRound)
if msg.Round == currentRound+1 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can have weird conditions where currentRound is 7, byt by the time you computed timeTillNextRound currentRound became 8 and timeTillNextRound is now a full round's time.
easiest option would be to first take a timeStamp (time.Now) and then do both computations against that timestamp.
I'd also consider having some "expressLAneParams" that holds initialTimeStamp and roundDuration and has methonds like: timeTillNextRound (which takes no params) and timeTillNextRoundAt (which takes timestamp as param)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the race condition. Not sure if making the params an object with methods makes much difference in clarity so I haven't done it for now.

timeboost.TimeTilNextRound(es.initialTimestamp, es.roundDuration) <= es.earlySubmissionGrace {
time.Sleep(timeboost.TimeTilNextRound(es.initialTimestamp, es.roundDuration))
tsahee marked this conversation as resolved.
Show resolved Hide resolved
} else {
return errors.Wrapf(timeboost.ErrBadRoundNumber, "express lane tx round %d does not match current round %d", msg.Round, currentRound)
}
}
if !es.currentRoundHasController() {
return timeboost.ErrNoOnchainController
Expand Down
56 changes: 51 additions & 5 deletions execution/gethexec/express_lane_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@ import (
"github.com/stretchr/testify/require"
)

var testPriv *ecdsa.PrivateKey
var testPriv, testPriv2 *ecdsa.PrivateKey

func init() {
privKey, err := crypto.HexToECDSA("93be75cc4df7acbb636b6abe6de2c0446235ac1dc7da9f290a70d83f088b486d")
if err != nil {
panic(err)
}
testPriv = privKey
privKey2, err := crypto.HexToECDSA("93be75cc4df7acbb636b6abe6de2c0446235ac1dc7da9f290a70d83f088b486e")
if err != nil {
panic(err)
}
testPriv2 = privKey2
}

func Test_expressLaneService_validateExpressLaneTx(t *testing.T) {
Expand Down Expand Up @@ -193,7 +198,7 @@ func Test_expressLaneService_validateExpressLaneTx(t *testing.T) {
control: expressLaneControl{
controller: common.Address{'b'},
},
sub: buildValidSubmission(t, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv),
sub: buildValidSubmission(t, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv, 0),
expectedErr: timeboost.ErrNotExpressLaneController,
},
{
Expand All @@ -210,7 +215,7 @@ func Test_expressLaneService_validateExpressLaneTx(t *testing.T) {
control: expressLaneControl{
controller: crypto.PubkeyToAddress(testPriv.PublicKey),
},
sub: buildValidSubmission(t, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv),
sub: buildValidSubmission(t, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv, 0),
valid: true,
},
}
Expand All @@ -231,6 +236,46 @@ func Test_expressLaneService_validateExpressLaneTx(t *testing.T) {
}
}

func Test_expressLaneService_validateExpressLaneTx_gracePeriod(t *testing.T) {
auctionContractAddr := common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6")
es := &expressLaneService{
auctionContractAddr: auctionContractAddr,
initialTimestamp: time.Now(),
roundDuration: time.Second * 10,
auctionClosing: time.Second * 5,
earlySubmissionGrace: time.Second * 2,
chainConfig: &params.ChainConfig{
ChainID: big.NewInt(1),
},
roundControl: lru.NewCache[uint64, *expressLaneControl](8),
}
es.roundControl.Add(0, &expressLaneControl{
controller: crypto.PubkeyToAddress(testPriv.PublicKey),
})
es.roundControl.Add(1, &expressLaneControl{
controller: crypto.PubkeyToAddress(testPriv2.PublicKey),
})

sub1 := buildValidSubmission(t, auctionContractAddr, testPriv, 0)
err := es.validateExpressLaneTx(sub1)
require.NoError(t, err)

// Send req for next round
sub2 := buildValidSubmission(t, auctionContractAddr, testPriv2, 1)
err = es.validateExpressLaneTx(sub2)
require.ErrorIs(t, err, timeboost.ErrBadRoundNumber)

// Sleep til 2 seconds before grace
time.Sleep(time.Second * 6)
err = es.validateExpressLaneTx(sub2)
require.ErrorIs(t, err, timeboost.ErrBadRoundNumber)

// Send req for next round within grace period
time.Sleep(time.Second * 2)
err = es.validateExpressLaneTx(sub2)
require.NoError(t, err)
}

type stubPublisher struct {
publishFn func(parentCtx context.Context, tx *types.Transaction, options *arbitrum_types.ConditionalOptions, delay bool) error
}
Expand Down Expand Up @@ -461,7 +506,7 @@ func Benchmark_expressLaneService_validateExpressLaneTx(b *testing.B) {
sequence: 1,
controller: addr,
})
sub := buildValidSubmission(b, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv)
sub := buildValidSubmission(b, common.HexToAddress("0x2Aef36410182881a4b13664a1E079762D7F716e6"), testPriv, 0)
b.StartTimer()
for i := 0; i < b.N; i++ {
err := es.validateExpressLaneTx(sub)
Expand Down Expand Up @@ -510,13 +555,14 @@ func buildValidSubmission(
t testing.TB,
auctionContractAddr common.Address,
privKey *ecdsa.PrivateKey,
round uint64,
) *timeboost.ExpressLaneSubmission {
b := &timeboost.ExpressLaneSubmission{
ChainId: big.NewInt(1),
AuctionContractAddress: auctionContractAddr,
Transaction: types.NewTransaction(0, common.Address{}, big.NewInt(0), 0, big.NewInt(0), nil),
Signature: make([]byte, 65),
Round: 0,
Round: round,
}
data, err := b.ToMessageBytes()
require.NoError(t, err)
Expand Down
13 changes: 11 additions & 2 deletions execution/gethexec/sequencer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type TimeboostConfig struct {
AuctioneerAddress string `koanf:"auctioneer-address"`
ExpressLaneAdvantage time.Duration `koanf:"express-lane-advantage"`
SequencerHTTPEndpoint string `koanf:"sequencer-http-endpoint"`
EarlySubmissionGrace time.Duration `koanf:"early-submission-grace"`
}

var DefaultTimeboostConfig = TimeboostConfig{
Expand All @@ -98,6 +99,7 @@ var DefaultTimeboostConfig = TimeboostConfig{
AuctioneerAddress: "",
ExpressLaneAdvantage: time.Millisecond * 200,
SequencerHTTPEndpoint: "http://localhost:8547",
EarlySubmissionGrace: time.Second * 2,
}

func (c *SequencerConfig) Validate() error {
Expand Down Expand Up @@ -191,6 +193,7 @@ func TimeboostAddOptions(prefix string, f *flag.FlagSet) {
f.String(prefix+".auctioneer-address", DefaultTimeboostConfig.AuctioneerAddress, "Address of the Timeboost Autonomous Auctioneer")
f.Duration(prefix+".express-lane-advantage", DefaultTimeboostConfig.ExpressLaneAdvantage, "specify the express lane advantage")
f.String(prefix+".sequencer-http-endpoint", DefaultTimeboostConfig.SequencerHTTPEndpoint, "this sequencer's http endpoint")
f.Duration(prefix+".early-submission-grace", DefaultTimeboostConfig.EarlySubmissionGrace, "period of time before the next round where submissions for the next round will be queued")
}

type txQueueItem struct {
Expand Down Expand Up @@ -482,7 +485,7 @@ func (s *Sequencer) publishTransactionImpl(parentCtx context.Context, tx *types.
}

if s.config().Timeboost.Enable && s.expressLaneService != nil {
if isExpressLaneController && s.expressLaneService.currentRoundHasController() {
if !isExpressLaneController && s.expressLaneService.currentRoundHasController() {
time.Sleep(s.config().Timeboost.ExpressLaneAdvantage)
}
}
Expand Down Expand Up @@ -1242,7 +1245,12 @@ func (s *Sequencer) Start(ctxIn context.Context) error {
return nil
}

func (s *Sequencer) StartExpressLane(ctx context.Context, auctionContractAddr common.Address, auctioneerAddr common.Address) {
func (s *Sequencer) StartExpressLane(
ctx context.Context,
auctionContractAddr common.Address,
auctioneerAddr common.Address,
earlySubmissionGrace time.Duration,
) {
if !s.config().Timeboost.Enable {
log.Crit("Timeboost is not enabled, but StartExpressLane was called")
}
Expand All @@ -1257,6 +1265,7 @@ func (s *Sequencer) StartExpressLane(ctx context.Context, auctionContractAddr co
auctionContractAddr,
seqClient,
s.execEngine.bc,
earlySubmissionGrace,
)
if err != nil {
log.Crit("Failed to create express lane service", "err", err, "auctionContractAddr", auctionContractAddr)
Expand Down
2 changes: 1 addition & 1 deletion system_tests/timeboost_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func setupExpressLaneAuction(
// This is hacky- we are manually starting the ExpressLaneService here instead of letting it be started
// by the sequencer. This is due to needing to deploy the auction contract first.
builderSeq.execConfig.Sequencer.Timeboost.Enable = true
builderSeq.L2.ExecNode.Sequencer.StartExpressLane(ctx, proxyAddr, seqInfo.GetAddress("AuctionContract"))
builderSeq.L2.ExecNode.Sequencer.StartExpressLane(ctx, proxyAddr, seqInfo.GetAddress("AuctionContract"), gethexec.DefaultTimeboostConfig.EarlySubmissionGrace)
t.Log("Started express lane service in sequencer")

// Set up an autonomous auction contract service that runs in the background in this test.
Expand Down
Loading