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 6 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
53 changes: 28 additions & 25 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,50 @@ 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


for hash := range decodedConstraints {
if !seenTx[hash] {
// The constraint is new, we will add this to the slot constraints
slotConstraints[hash] = decodedConstraints[hash]
Comment on lines +387 to +390
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we iterate over the decoded constraints and see if any of the constraints present in it is different and if so we add it to the slotConstraints

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

// Update the slot constraints
b.constraintsCache.Put(constraint.Message.Slot, slotConstraints)
Comment on lines +394 to +395
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We put the updated slot constraints in the cache. If there are no changes to it, we put the same back

}
}

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
24 changes: 18 additions & 6 deletions builder/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,13 +627,25 @@ 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)
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)
for _, slot := range slots {
cachedConstraints, ok := builder.constraintsCache.Get(slot)
require.Equal(t, true, ok)

expectedConstraint := generateMockConstraintsForSlot(slot)[0]
decodedConstraint, err := DecodeConstraint(expectedConstraint)
require.NoError(t, err)

// Compare the keys of the cachedConstraints and decodedConstraint maps
require.Equal(t, len(cachedConstraints), len(decodedConstraint), "The number of keys in both maps should be the same")
for key := range cachedConstraints {
_, ok := decodedConstraint[key]
require.True(t, ok, fmt.Sprintf("Key %s found in cachedConstraints but not in decodedConstraint", key.String()))
require.Equal(t, cachedConstraints[key].Tx.Data(), decodedConstraint[key].Tx.Data(), "The decodedConstraint Tx should be equal to the cachedConstraints Tx")
}
for key := range decodedConstraint {
_, ok := cachedConstraints[key]
require.True(t, ok, fmt.Sprintf("Key %s found in decodedConstraint but not in cachedConstraints", key.String()))
}
}
}

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