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.

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

feat(octane/evmengine): fetch eligible withdrawals for block inclusion #3099

Merged
merged 4 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions halo/app/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ const (

deliverIntervalProtected = 20_000 // Roughly ~12h assuming 0.5bps
deliverIntervalEphemeral = 2 // Fast updates while testing

maxWithdrawalsPerBlock uint64 = 32 // The maximum number of withdrawals included in one block.
)

//nolint:gochecknoglobals // Cosmos-style
Expand Down Expand Up @@ -202,8 +204,10 @@ var (
}),
},
{
Name: engevmtypes.ModuleName,
Config: appconfig.WrapAny(&engevmmodule.Module{}),
Name: engevmtypes.ModuleName,
Config: appconfig.WrapAny(&engevmmodule.Module{
MaxWithdrawalsPerBlock: maxWithdrawalsPerBlock,
}),
},
{
Name: attesttypes.ModuleName,
Expand Down
14 changes: 8 additions & 6 deletions octane/evmengine/keeper/abci_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import (
var zeroAddr common.Address
var zeroHash common.Hash

const maxWithdrawalsPerBlock uint64 = 32

func TestKeeper_PrepareProposal(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -129,7 +131,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: tutil.RandomAddress(),
}
frp := newRandomFeeRecipientProvider()
k, err := NewKeeper(cdc, storeService, &tt.mockEngine, txConfig, ap, frp)
k, err := NewKeeper(cdc, storeService, &tt.mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
populateGenesisHead(ctx, t, k)

Expand Down Expand Up @@ -157,7 +159,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -220,7 +222,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -291,7 +293,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -348,7 +350,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -407,7 +409,7 @@ func TestOptimistic(t *testing.T) {
}

frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
keeper.SetCometAPI(cmtAPI)
Expand Down
35 changes: 35 additions & 0 deletions octane/evmengine/keeper/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/omni-network/omni/lib/cast"
"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/umath"

"github.com/ethereum/go-ethereum/beacon/engine"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -113,3 +114,37 @@ func (k *Keeper) listWithdrawalsByAddress(ctx context.Context, withdrawalAddr co

return withdrawals, nil
}

// EligibleWithdrawals returns all withdrawals created below the specified height,
// sorted by the id (oldest to newest), limited by the provided count.
func (k *Keeper) EligibleWithdrawals(ctx context.Context) ([]*Withdrawal, error) {
height := sdk.UnwrapSDKContext(ctx).BlockHeight()
// Note: items are ordered by the id in ascending order (oldest to newest).
iter, err := k.withdrawalTable.List(ctx, WithdrawalPrimaryKey{})
if err != nil {
return nil, errors.Wrap(err, "list withdrawals")
}
defer iter.Close()

var withdrawals []*Withdrawal
for iter.Next() {
val, err := iter.Value()
if err != nil {
return nil, errors.Wrap(err, "get withdrawal")
}

if val.GetCreatedHeight() >= uint64(height) {
// Withdrawals created in this block are not eligible
break
}

withdrawals = append(withdrawals, val)

if umath.Len(withdrawals) == k.maxWithdrawalsPerBlock {
// Reached the max number of withdrawals
break
}
}

return withdrawals, nil
}
64 changes: 56 additions & 8 deletions octane/evmengine/keeper/db_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,26 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {
}
frp := newRandomFeeRecipientProvider()
evmLogProc := mockEventProc{deliverErr: errors.New("test error")}
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, evmLogProc)
maxWithdrawalsPerBlock := uint64(4)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, evmLogProc)
require.NoError(t, err)

addr1 := tutil.RandomAddress()
addr2 := tutil.RandomAddress()
addr3 := tutil.RandomAddress()

inputs := []struct {
type testCase struct {
addr common.Address
height uint64
amount uint64
expID uint64
}{
}

inputs := []testCase{
{addr1, 1, 777, 1},
{addr2, 2, 8888, 2},
{addr1, 100, 9999999, 3},
{addr3, 120, 10000000, 4},
}

for _, in := range inputs {
Expand All @@ -63,13 +68,17 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {

withdrawals, err := getAllWithdrawals(ctx, keeper)
require.NoError(t, err)
require.Len(t, withdrawals, 3)
require.Len(t, withdrawals, len(inputs))

matchesTestCase := func(w *Withdrawal, in testCase) {
require.Equal(t, in.expID, w.GetId())
require.Equal(t, in.addr.Bytes(), w.GetAddress())
require.Equal(t, in.amount, w.GetAmountGwei())
require.Equal(t, in.height, w.GetCreatedHeight())
}

for i, in := range inputs {
require.Equal(t, in.expID, withdrawals[i].GetId())
require.Equal(t, in.addr.Bytes(), withdrawals[i].GetAddress())
require.Equal(t, in.amount, withdrawals[i].GetAmountGwei())
require.Equal(t, in.height, withdrawals[i].GetCreatedHeight())
matchesTestCase(withdrawals[i], in)
}

withdrawalsByAddr, err := keeper.listWithdrawalsByAddress(ctx, addr1)
Expand All @@ -85,6 +94,45 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {
withdrawalsByAddr, err = keeper.listWithdrawalsByAddress(ctx, tutil.RandomAddress())
require.NoError(t, err)
require.Empty(t, withdrawalsByAddr)

// make sure we have no withdrawals below and at height 1
withdrawalsByHeight, err := keeper.EligibleWithdrawals(ctx.WithBlockHeight(0))
require.NoError(t, err)
require.Empty(t, withdrawalsByHeight)

withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1))
require.NoError(t, err)
require.Empty(t, withdrawalsByHeight)

// make sure we have exactly one withdrawal below height 2
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(2))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 1)
matchesTestCase(withdrawalsByHeight[0], inputs[0])

// under height 50 we only have 2 withdrawals
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(50))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 2)
matchesTestCase(withdrawalsByHeight[0], inputs[0])
matchesTestCase(withdrawalsByHeight[1], inputs[1])

// under height 1000 we get all of them
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1000))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 4)
matchesTestCase(withdrawalsByHeight[0], inputs[0])
matchesTestCase(withdrawalsByHeight[1], inputs[1])
matchesTestCase(withdrawalsByHeight[2], inputs[2])
matchesTestCase(withdrawalsByHeight[3], inputs[3])

// under height 1000 we get the first 2 if we limit the output by 2
keeper.maxWithdrawalsPerBlock /= 2
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1000))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, int(keeper.maxWithdrawalsPerBlock))
matchesTestCase(withdrawalsByHeight[0], inputs[0])
matchesTestCase(withdrawalsByHeight[1], inputs[1])
}

// getAllWithdrawals returns all withdrawals in the keeper DB.
Expand Down
47 changes: 25 additions & 22 deletions octane/evmengine/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,20 @@ import (
)

type Keeper struct {
cdc codec.BinaryCodec
storeService store.KVStoreService
headTable ExecutionHeadTable
withdrawalTable WithdrawalTable
engineCl ethclient.EngineClient
txConfig client.TxConfig
voteProvider types.VoteExtensionProvider
eventProcs []types.EvmEventProcessor
cmtAPI comet.API
addrProvider types.AddressProvider
feeRecProvider types.FeeRecipientProvider
buildDelay time.Duration
buildOptimistic bool
cdc codec.BinaryCodec
storeService store.KVStoreService
headTable ExecutionHeadTable
withdrawalTable WithdrawalTable
engineCl ethclient.EngineClient
txConfig client.TxConfig
voteProvider types.VoteExtensionProvider
eventProcs []types.EvmEventProcessor
cmtAPI comet.API
addrProvider types.AddressProvider
feeRecProvider types.FeeRecipientProvider
buildDelay time.Duration
buildOptimistic bool
maxWithdrawalsPerBlock uint64

// mutablePayload contains the previous optimistically triggered payload.
// It is optimistic because the validator set can change,
Expand All @@ -57,6 +58,7 @@ func NewKeeper(
txConfig client.TxConfig,
addrProvider types.AddressProvider,
feeRecProvider types.FeeRecipientProvider,
maxWithdrawalsPerBlock uint64,
eventProcs ...types.EvmEventProcessor,
) (*Keeper, error) {
schema := &ormv1alpha1.ModuleSchemaDescriptor{SchemaFile: []*ormv1alpha1.ModuleSchemaDescriptor_FileEntry{
Expand All @@ -78,15 +80,16 @@ func NewKeeper(
}

return &Keeper{
cdc: cdc,
storeService: storeService,
headTable: dbStore.ExecutionHeadTable(),
withdrawalTable: dbStore.WithdrawalTable(),
engineCl: engineCl,
txConfig: txConfig,
addrProvider: addrProvider,
feeRecProvider: feeRecProvider,
eventProcs: eventProcs,
cdc: cdc,
storeService: storeService,
headTable: dbStore.ExecutionHeadTable(),
withdrawalTable: dbStore.WithdrawalTable(),
engineCl: engineCl,
txConfig: txConfig,
addrProvider: addrProvider,
feeRecProvider: feeRecProvider,
eventProcs: eventProcs,
maxWithdrawalsPerBlock: maxWithdrawalsPerBlock,
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/keeper_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ func TestKeeper_isNextProposer(t *testing.T) {
address: nxtAddr,
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
keeper.SetCometAPI(cmtAPI)
populateGenesisHead(ctx, t, keeper)
Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/msg_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func Test_msgServer_ExecutionPayload(t *testing.T) {
}
frp := newRandomFeeRecipientProvider()
evmLogProc := mockEventProc{deliverErr: errors.New("test error")}
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, evmLogProc)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, evmLogProc)
require.NoError(t, err)
keeper.SetCometAPI(cmtAPI)
populateGenesisHead(ctx, t, keeper)
Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/proposal_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func Test_proposalServer_ExecutionPayload(t *testing.T) {
sdkCtx = sdkCtx.WithExecMode(sdk.ExecModeFinalize)

frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, nil, frp)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, nil, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
populateGenesisHead(sdkCtx, t, keeper)

Expand Down
1 change: 1 addition & 0 deletions octane/evmengine/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ func ProvideModule(in ModuleInputs) (ModuleOutputs, error) {
in.TXConfig,
in.AddrProvider,
in.FeeRecProvider,
in.Config.GetMaxWithdrawalsPerBlock(),
eventProcs...,
)
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion octane/evmengine/module/module.proto
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ message Module {

// authority defines the custom module authority. If not set, defaults to the governance module.
string authority = 1;
}

// max_withdrawals_per_block specifies the maximum number of withdrawals included in one block.
uint64 max_withdrawals_per_block = 2;
}
Loading
Loading