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

consensus/bor: eliminate contract calls for current span #1444

Closed
wants to merge 9 commits into from
70 changes: 46 additions & 24 deletions consensus/bor/bor.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ const (
inmemorySignatures = 4096 // Number of recent block signatures to keep in memory
)

const (
defaultSpanLength = 6400 // Default span length i.e. number of bor blocks in a span
zerothSpanEnd = 255 // End block of 0th span
)

// Bor protocol constants.
var (
defaultSprintLength = map[string]uint64{
Expand Down Expand Up @@ -228,6 +233,8 @@ type Bor struct {
fakeDiff bool // Skip difficulty verifications
devFakeAuthor bool

spanLength uint64 // Span length populated by heimdall

closeOnce sync.Once
}

Expand Down Expand Up @@ -285,6 +292,16 @@ func New(
}
}

// Fetch bor params from heimdall to populate span length
borParams, err := heimdallClient.BorParams(context.Background())
if err != nil || borParams == nil || borParams.SpanLength == 0 {
log.Warn("Unable to fetch bor params or invalid params received, using default span length", "length", defaultSpanLength)
c.spanLength = defaultSpanLength
} else {
c.spanLength = borParams.SpanLength
log.Debug("Populated span length from heimdall", "length", c.spanLength)
}

return c
}

Expand Down Expand Up @@ -1094,39 +1111,44 @@ func (c *Bor) Close() error {
return nil
}

func (c *Bor) checkAndCommitSpan(
state *state.StateDB,
header *types.Header,
chain core.ChainContext,
) error {
var ctx = context.Background()
headerNumber := header.Number.Uint64()
// SpanIdAt returns the corresponding span id for the given block number.
func (c *Bor) SpanIdAt(blockNum uint64) uint64 {
if blockNum > zerothSpanEnd {
return 1 + (blockNum-zerothSpanEnd-1)/c.spanLength
}
return 0
}

span, err := c.spanner.GetCurrentSpan(ctx, header.ParentHash)
if err != nil {
return err
// SpanEndBlockNum returns the number of the last block in the given span.
func (c *Bor) SpanEndBlockNum(spanId uint64) uint64 {
if spanId > 0 {
return spanId*c.spanLength + zerothSpanEnd
}
return zerothSpanEnd
}

if c.needToCommitSpan(span, headerNumber) {
return c.FetchAndCommitSpan(ctx, span.ID+1, state, header, chain)
func (c *Bor) checkAndCommitSpan(state *state.StateDB, header *types.Header, chain core.ChainContext) error {
// Find the current span at parent block
headerNumber := header.Number.Uint64()
spanId := c.SpanIdAt(headerNumber)
sprintLength := c.config.CalculateSprint(headerNumber)

// Fetch the next span if required
if c.needToCommitSpan(spanId, headerNumber, sprintLength) {
return c.FetchAndCommitSpan(context.Background(), spanId+1, state, header, chain)
}

return nil
}

func (c *Bor) needToCommitSpan(currentSpan *span.Span, headerNumber uint64) bool {
// if span is nil
if currentSpan == nil {
return false
}

// check span is not set initially
if currentSpan.EndBlock == 0 {
func (c *Bor) needToCommitSpan(spanId, headerNumber, sprintLength uint64) bool {
// Find the end block of the given span
spanEndBlock := c.SpanEndBlockNum(spanId)
if spanId == 0 && headerNumber == sprintLength {
// when in span 0 we fetch the next span (span 1) at the beginning of sprint 2 (block 16)
return true
}

// if current block is first block of last sprint in current span
if currentSpan.EndBlock > c.config.CalculateSprint(headerNumber) && currentSpan.EndBlock-c.config.CalculateSprint(headerNumber)+1 == headerNumber {
} else if spanEndBlock-sprintLength+1 == headerNumber {
// for subsequent spans, we always fetch the next span at the beginning of the last sprint of current span
return true
}

Expand Down
96 changes: 96 additions & 0 deletions consensus/bor/bor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bor

import (
"math/big"
"strconv"
"testing"

"github.com/holiman/uint256"
Expand Down Expand Up @@ -162,3 +163,98 @@ func TestEncodeSigHeaderJaipur(t *testing.T) {
hash = SealHash(h, &params.BorConfig{JaipurBlock: big.NewInt(10)})
require.Equal(t, hash, hashWithoutBaseFee)
}

func TestNeedToCommitSpan(t *testing.T) {
type test struct {
spanId uint64
headerNumber uint64
want bool
}

// Create a minimalistic fake bor consensus instance
bor := &Bor{spanLength: 6400}

tests := []test{
{spanId: 0, headerNumber: 1, want: false},
{spanId: 0, headerNumber: 15, want: false},
{spanId: 0, headerNumber: 16, want: true}, // Second sprint start
{spanId: 0, headerNumber: 17, want: false},
{spanId: 1, headerNumber: 256, want: false},
{spanId: 1, headerNumber: 6639, want: false},
{spanId: 1, headerNumber: 6640, want: true}, // First block of last sprint of span 1
{spanId: 1, headerNumber: 6641, want: false},
{spanId: 1, headerNumber: 6655, want: false},
{spanId: 100, headerNumber: 633856, want: false},
{spanId: 100, headerNumber: 640239, want: false},
{spanId: 100, headerNumber: 640240, want: true}, // First block of last sprint of span 100
{spanId: 100, headerNumber: 640241, want: false},
}

for _, test := range tests {
test := test
name := "id=" + strconv.FormatUint(test.spanId, 10) + ",number=" + strconv.FormatUint(test.headerNumber, 10)
t.Run(name, func(t *testing.T) {
t.Parallel()
require.Equal(t, test.want, bor.needToCommitSpan(test.spanId, test.headerNumber, 16))
})
}
}

func TestSpanIdAt(t *testing.T) {
type test struct {
blockNumber uint64
want uint64
}

// Create a minimalistic fake bor consensus instance
bor := &Bor{spanLength: 6400}

tests := []test{
{blockNumber: 0, want: 0},
{blockNumber: 1, want: 0},
{blockNumber: 255, want: 0},
{blockNumber: 256, want: 1},
{blockNumber: 6655, want: 1},
{blockNumber: 6656, want: 2},
{blockNumber: 1632256, want: 256},
{blockNumber: 1638655, want: 256},
{blockNumber: 1635456, want: 256},
}

for _, test := range tests {
test := test
name := "number=" + strconv.FormatUint(test.blockNumber, 10)
t.Run(name, func(t *testing.T) {
t.Parallel()
require.Equal(t, test.want, bor.SpanIdAt(test.blockNumber))
})
}
}

func TestSpanEndBlockNum(t *testing.T) {
type test struct {
spanId uint64
want uint64
}

// Create a minimalistic fake bor consensus instance
bor := &Bor{spanLength: 6400}

tests := []test{
{spanId: 0, want: 255},
{spanId: 1, want: 6655},
{spanId: 2, want: 13055},
{spanId: 100, want: 640255},
{spanId: 256, want: 1638655},
{spanId: 1000, want: 6400255},
}

for _, test := range tests {
test := test
name := "number=" + strconv.FormatUint(test.spanId, 10)
t.Run(name, func(t *testing.T) {
t.Parallel()
require.Equal(t, test.want, bor.SpanEndBlockNum(test.spanId))
})
}
}
2 changes: 2 additions & 0 deletions consensus/bor/heimdall.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
"github.com/ethereum/go-ethereum/consensus/bor/clerk"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/milestone"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/params"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/span"
)

//go:generate mockgen -destination=../../tests/bor/mocks/IHeimdallClient.go -package=mocks . IHeimdallClient
type IHeimdallClient interface {
BorParams(ctx context.Context) (*params.BorParams, error)
StateSyncEvents(ctx context.Context, fromID uint64, to int64) ([]*clerk.EventRecordWithTime, error)
Span(ctx context.Context, spanID uint64) (*span.HeimdallSpan, error)
FetchCheckpoint(ctx context.Context, number int64) (*checkpoint.Checkpoint, error)
Expand Down
23 changes: 23 additions & 0 deletions consensus/bor/heimdall/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ethereum/go-ethereum/consensus/bor/clerk"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/milestone"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/params"
"github.com/ethereum/go-ethereum/consensus/bor/heimdall/span"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/metrics"
Expand Down Expand Up @@ -84,8 +85,26 @@ const (
fetchMilestoneID = "/milestone/ID/%s"

fetchSpanFormat = "bor/span/%d"

fetchBorParams = "bor/params"
)

func (h *HeimdallClient) BorParams(ctx context.Context) (*params.BorParams, error) {
url, err := borParamsURL(h.urlString)
if err != nil {
return nil, err
}

ctx = withRequestType(ctx, borParamsRequest)

response, err := FetchWithRetry[params.BorParamsResponse](ctx, h.client, url, h.closeCh)
if err != nil {
return nil, err
}

return &response.Result, nil
}

func (h *HeimdallClient) StateSyncEvents(ctx context.Context, fromID uint64, to int64) ([]*clerk.EventRecordWithTime, error) {
eventRecords := make([]*clerk.EventRecordWithTime, 0)

Expand Down Expand Up @@ -367,6 +386,10 @@ func Fetch[T any](ctx context.Context, request *Request) (*T, error) {
return result, nil
}

func borParamsURL(urlString string) (*url.URL, error) {
return makeURL(urlString, fetchBorParams, "")
}

func spanURL(urlString string, spanID uint64) (*url.URL, error) {
return makeURL(urlString, fmt.Sprintf(fetchSpanFormat, spanID), "")
}
Expand Down
1 change: 1 addition & 0 deletions consensus/bor/heimdall/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type (
)

const (
borParamsRequest requestType = "bor-params"
stateSyncRequest requestType = "state-sync"
spanRequest requestType = "span"
checkpointRequest requestType = "checkpoint"
Expand Down
12 changes: 12 additions & 0 deletions consensus/bor/heimdall/params/bor_params.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package params

type BorParamsResponse struct {
Height string `json:"height"`
Result BorParams `json:"result"`
}

type BorParams struct {
SprintLength uint64 `json:"sprint_duration"`
SpanLength uint64 `json:"span_duration"`
ProducerCount uint64 `json:"producer_count"`
}
3 changes: 3 additions & 0 deletions consensus/bor/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ func (s *Snapshot) apply(headers []*types.Header, c *Bor) (*Snapshot, error) {
// add recents
snap.Recents[number] = signer

// TODO
// Can't we do this step for the last header only?

// change validator set and change proposer
if number > 0 && (number+1)%s.chainConfig.Bor.CalculateSprint(number) == 0 {
if err := validateHeaderExtraField(header.Extra); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion tests/bor/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func getMockedSpanner(t *testing.T, validators []*valset.Validator) *bor.MockSpa
spanner := bor.NewMockSpanner(gomock.NewController(t))
spanner.EXPECT().GetCurrentValidatorsByHash(gomock.Any(), gomock.Any(), gomock.Any()).Return(validators, nil).AnyTimes()
spanner.EXPECT().GetCurrentValidatorsByBlockNrOrHash(gomock.Any(), gomock.Any(), gomock.Any()).Return(validators, nil).AnyTimes()
spanner.EXPECT().GetCurrentSpan(gomock.Any(), gomock.Any()).Return(&span.Span{0, 0, 0}, nil).AnyTimes()
spanner.EXPECT().GetCurrentSpan(gomock.Any(), gomock.Any()).Return(&span.Span{0, 0, 0}, nil).MaxTimes(0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi, this change makes sure that GetCurrentSpan isn't called even once and the tests still works suggesting that the new implementation is replacable.

spanner.EXPECT().CommitSpan(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
return spanner
}
Expand Down
16 changes: 16 additions & 0 deletions tests/bor/mocks/IHeimdallClient.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading