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

chore(builder): store decoded constraints in the cache #68

Merged
merged 8 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
56 changes: 33 additions & 23 deletions builder/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ type Builder struct {
builderResubmitInterval time.Duration
discardRevertibleTxOnErr bool

// constraintsCache is a map from slot to the constraints made by proposers
constraintsCache *shardmap.FIFOMap[uint64, common.SignedConstraintsList]
// constraintsCache is a map from slot to the decoded constraints made by proposers
constraintsCache *shardmap.FIFOMap[uint64, types.HashToConstraintDecoded]

limiter *rate.Limiter
submissionOffsetFromEndOfSlot time.Duration
Expand Down Expand Up @@ -196,7 +196,7 @@ func NewBuilder(args BuilderArgs) (*Builder, error) {
discardRevertibleTxOnErr: args.discardRevertibleTxOnErr,
submissionOffsetFromEndOfSlot: args.submissionOffsetFromEndOfSlot,

constraintsCache: shardmap.NewFIFOMap[uint64, common.SignedConstraintsList](64, 16, shardmap.HashUint64),
constraintsCache: shardmap.NewFIFOMap[uint64, types.HashToConstraintDecoded](64, 16, shardmap.HashUint64),

limiter: args.limiter,
slotCtx: slotCtx,
Expand Down Expand Up @@ -343,9 +343,11 @@ func (b *Builder) subscribeToRelayForConstraints(relayBaseEndpoint, authHeader s
}
log.Error("Error reading from response body: %v", err)
}

if !strings.HasPrefix(line, "data: ") {
continue
}

data := strings.TrimPrefix(line, "data: ")

// We assume the data is the JSON representation of the constraints
Expand All @@ -355,49 +357,57 @@ func (b *Builder) subscribeToRelayForConstraints(relayBaseEndpoint, authHeader s
log.Warn(fmt.Sprintf("Failed to unmarshal constraints: %v", err))
continue
}

if len(constraintsSigned) == 0 {
log.Warn("Received 0 length list of constraints")
continue
}

OUTER:
for _, constraint := range constraintsSigned {
decodedConstraints, err := DecodeConstraint(constraint)
if err != nil {
log.Error("Failed to decode transaction RLP: ", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Error("Failed to decode transaction RLP: ", err)
log.Error("Failed to decode constraint: ", err)

continue
}
Comment on lines +367 to +371
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decode the constraints from the function in utils


// For every constraint, we need to check if it has already been seen for the associated slot
slotConstraints, _ := b.constraintsCache.Get(constraint.Message.Slot)
if len(slotConstraints) == 0 {
// New constraint for this slot, add it in the map and continue with the next constraint
b.constraintsCache.Put(constraint.Message.Slot, common.SignedConstraintsList{constraint})
b.constraintsCache.Put(constraint.Message.Slot, decodedConstraints)
continue
}
for _, slotConstraint := range slotConstraints {
if slotConstraint.Signature == constraint.Signature {
// The constraint has already been seen, we can continue with the next one
continue OUTER

// Temporary map to keep track of the constraints that are already in the slot
seenTx := make(map[common.Hash]bool)
for hash := range slotConstraints {
seenTx[hash] = true
}
Comment on lines +381 to +385
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need this additional hashmap, you can just check if slotConstraint[hash] == nil, or writing every without doing this check


isNewConstraint := false
for hash := range decodedConstraints {
if !seenTx[hash] {
// The constraint is new, we will add this to the slot constraints
isNewConstraint = true
slotConstraints[hash] = decodedConstraints[hash]
}
}

if !isNewConstraint {
continue OUTER
}

// The constraint is new, we need to append it to the current list
b.constraintsCache.Put(constraint.Message.Slot, append(slotConstraints, constraint))
b.constraintsCache.Put(constraint.Message.Slot, slotConstraints)
}
}

return nil
}

func (b *Builder) GetConstraintsForSlot(slot uint64) types.HashToConstraintDecoded {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can just remove this function now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, removed it!

constraintsDecoded := make(types.HashToConstraintDecoded)
constraintsSigned, _ := b.constraintsCache.Get(slot)

for _, constraintSigned := range constraintsSigned {
constraints := constraintSigned.Message.Constraints
for _, constraint := range constraints {
decoded := new(types.Transaction)
if err := decoded.UnmarshalBinary(constraint.Tx); err != nil {
log.Error("Failed to decode preconfirmation transaction RLP: ", err)
continue
}
constraintsDecoded[decoded.Hash()] = &types.ConstraintDecoded{Index: constraint.Index, Tx: decoded}
}
}
constraintsDecoded, _ := b.constraintsCache.Get(slot)
return constraintsDecoded
}

Expand Down
10 changes: 5 additions & 5 deletions builder/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,12 +627,12 @@ func TestSubscribeProposerConstraints(t *testing.T) {
time.Sleep(2 * time.Second)

slots := []uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
for slot := range slots {
slot := uint64(slot)
for _, slot := range slots {
constraints, ok := builder.constraintsCache.Get(slot)
expected := generateMockConstraintsForSlot(slot)[0].Message.Constraints[0].Tx
actual := constraints[0].Message.Constraints[0].Tx
require.Equal(t, expected, actual)
expected := generateMockConstraintsForSlot(slot)[0]
decoded, _ := DecodeConstraint(expected)
// actual := constraints[0].Message.Constraints[0].Tx
require.Equal(t, constraints, decoded)
require.Equal(t, true, ok)
}
}
Expand Down
15 changes: 15 additions & 0 deletions builder/builder/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,25 @@ import (
"fmt"
"io"
"net/http"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
)

var errHTTPErrorResponse = errors.New("HTTP error response")

func DecodeConstraint(constraint *common.SignedConstraints) (types.HashToConstraintDecoded, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func DecodeConstraint(constraint *common.SignedConstraints) (types.HashToConstraintDecoded, error) {
func DecodeConstraints(constraints *common.SignedConstraints) (types.HashToConstraintDecoded, error) {

decodedConstraints := make(types.HashToConstraintDecoded)
for _, tx := range constraint.Message.Constraints {
decoded := new(types.Transaction)
if err := decoded.UnmarshalBinary(tx.Tx); err != nil {
return nil, err
}
decodedConstraints[decoded.Hash()] = &types.ConstraintDecoded{Index: tx.Index, Tx: decoded}
}
return decodedConstraints, nil
}

// SendSSZRequest is a request to send SSZ data to a remote relay.
func SendSSZRequest(ctx context.Context, client http.Client, method, url string, payload []byte, useGzip bool) (code int, err error) {
var req *http.Request
Expand Down
6 changes: 3 additions & 3 deletions builder/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
github.com/btcsuite/btcd/btcec/v2 v2.2.1
github.com/cenkalti/backoff/v4 v4.2.1
github.com/cespare/cp v0.1.0
github.com/chainbound/shardmap v0.0.2
github.com/cloudflare/cloudflare-go v0.79.0
github.com/cockroachdb/pebble v0.0.0-20230928194634-aa077af62593
github.com/consensys/gnark-crypto v0.12.1
Expand All @@ -39,6 +40,7 @@ require (
github.com/golang/snappy v0.0.5-0.20220116011046-fa5810519dcb
github.com/google/gofuzz v1.2.0
github.com/google/uuid v1.3.0
github.com/gorilla/handlers v1.5.2
github.com/gorilla/mux v1.8.0
github.com/gorilla/websocket v1.4.2
github.com/grafana/pyroscope-go/godeltaprof v0.1.7
Expand Down Expand Up @@ -84,11 +86,9 @@ require (
)

require (
github.com/chainbound/shardmap v0.0.2 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/getsentry/sentry-go v0.18.0 // indirect
github.com/goccy/go-yaml v1.11.2 // indirect
github.com/gorilla/handlers v1.5.2 // indirect
github.com/klauspost/cpuid/v2 v2.2.5 // indirect
github.com/minio/sha256-simd v1.0.1 // indirect
github.com/naoina/go-stringutil v0.1.0 // indirect
Expand Down Expand Up @@ -150,7 +150,7 @@ require (
github.com/mmcloughlin/addchain v0.4.0 // indirect
github.com/naoina/toml v0.1.1
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pkg/errors v0.9.1
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.16.0 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
Expand Down