From e6571ca397c43642dda0dc6da31140c16a3a5013 Mon Sep 17 00:00:00 2001 From: Antonio Nunez Date: Fri, 15 Nov 2024 17:55:28 +0800 Subject: [PATCH 01/10] feat: update google-cloud-storage parameters and object retention setting --- das/google_cloud_storage_service.go | 173 +++++++++------------------- 1 file changed, 57 insertions(+), 116 deletions(-) diff --git a/das/google_cloud_storage_service.go b/das/google_cloud_storage_service.go index 829f4b5265..adf039347c 100644 --- a/das/google_cloud_storage_service.go +++ b/das/google_cloud_storage_service.go @@ -21,151 +21,88 @@ import ( "github.com/offchainlabs/nitro/util/pretty" ) -type GoogleCloudStorageOperator interface { - Bucket(name string) *googlestorage.BucketHandle - Upload(ctx context.Context, bucket, objectPrefix string, value []byte) error - Download(ctx context.Context, bucket, objectPrefix string, key common.Hash) ([]byte, error) - Close(ctx context.Context) error -} - -type GoogleCloudStorageClient struct { - client *googlestorage.Client -} - -func (g *GoogleCloudStorageClient) Bucket(name string) *googlestorage.BucketHandle { - return g.client.Bucket(name) -} - -func (g *GoogleCloudStorageClient) Upload(ctx context.Context, bucket, objectPrefix string, value []byte) error { - obj := g.client.Bucket(bucket).Object(objectPrefix + EncodeStorageServiceKey(dastree.Hash(value))) - w := obj.NewWriter(ctx) - - if _, err := fmt.Fprintln(w, value); err != nil { - return err - } - return w.Close() - -} - -func (g *GoogleCloudStorageClient) Download(ctx context.Context, bucket, objectPrefix string, key common.Hash) ([]byte, error) { - obj := g.client.Bucket(bucket).Object(objectPrefix + EncodeStorageServiceKey(key)) - reader, err := obj.NewReader(ctx) - if err != nil { - return nil, err - } - return io.ReadAll(reader) -} - -func (g *GoogleCloudStorageClient) Close(ctx context.Context) error { - return g.client.Close() -} - type GoogleCloudStorageServiceConfig struct { - Enable bool `koanf:"enable"` - AccessToken string `koanf:"access-token"` - Bucket string `koanf:"bucket"` - ObjectPrefix string `koanf:"object-prefix"` - EnableExpiry bool `koanf:"enable-expiry"` - MaxRetention time.Duration `koanf:"max-retention"` + Enable bool `koanf:"enable"` + AccessTokenFile string `koanf:"access-token-file"` + Bucket string `koanf:"bucket"` + ObjectPrefix string `koanf:"object-prefix"` + DiscardAfterTimeout bool `koanf:"discard-after-timeout"` } var DefaultGoogleCloudStorageServiceConfig = GoogleCloudStorageServiceConfig{} func GoogleCloudConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".enable", DefaultGoogleCloudStorageServiceConfig.Enable, "EXPERIMENTAL/unsupported - enable storage/retrieval of sequencer batch data from an Google Cloud Storage bucket") - f.String(prefix+".access-token", DefaultGoogleCloudStorageServiceConfig.AccessToken, "Google Cloud Storage access token") + f.String(prefix+".access-token-file", DefaultGoogleCloudStorageServiceConfig.AccessTokenFile, "Google Cloud Storage access token") f.String(prefix+".bucket", DefaultGoogleCloudStorageServiceConfig.Bucket, "Google Cloud Storage bucket") f.String(prefix+".object-prefix", DefaultGoogleCloudStorageServiceConfig.ObjectPrefix, "prefix to add to Google Cloud Storage objects") - f.Bool(prefix+".enable-expiry", DefaultLocalFileStorageConfig.EnableExpiry, "enable expiry of batches") - f.Duration(prefix+".max-retention", DefaultLocalFileStorageConfig.MaxRetention, "store requests with expiry times farther in the future than max-retention will be rejected") + f.Bool(prefix+".discard-after-timeout", DefaultGoogleCloudStorageServiceConfig.DiscardAfterTimeout, "discard data after its expiry timeout") } type GoogleCloudStorageService struct { - operator GoogleCloudStorageOperator - bucket string - objectPrefix string - enableExpiry bool - maxRetention time.Duration + client *googlestorage.Client + bucket string + objectPrefix string + discardAfterTimeout bool } func NewGoogleCloudStorageService(config GoogleCloudStorageServiceConfig) (StorageService, error) { - var client *googlestorage.Client - var err error - // Note that if the credentials are not specified, the client library will find credentials using ADC(Application Default Credentials) - // https://cloud.google.com/docs/authentication/provide-credentials-adc. - if config.AccessToken == "" { - client, err = googlestorage.NewClient(context.Background()) - } else { - client, err = googlestorage.NewClient(context.Background(), option.WithCredentialsJSON([]byte(config.AccessToken))) - } + client, err := buildGoogleCloudStorageClient(config.AccessTokenFile) if err != nil { return nil, fmt.Errorf("error creating Google Cloud Storage client: %w", err) } - service := &GoogleCloudStorageService{ - operator: &GoogleCloudStorageClient{client: client}, - bucket: config.Bucket, - objectPrefix: config.ObjectPrefix, - enableExpiry: config.EnableExpiry, - maxRetention: config.MaxRetention, - } - if config.EnableExpiry { - lifecycleRule := googlestorage.LifecycleRule{ - Action: googlestorage.LifecycleAction{Type: "Delete"}, - Condition: googlestorage.LifecycleCondition{AgeInDays: int64(config.MaxRetention.Hours() / 24)}, // Objects older than 30 days - } - ctx := context.Background() - bucket := service.operator.Bucket(service.bucket) - // check if bucket exists (and others), and update expiration policy if enabled - attrs, err := bucket.Attrs(ctx) - if err != nil { - return nil, fmt.Errorf("error getting bucket attributes: %w", err) - } - attrs.Lifecycle.Rules = append(attrs.Lifecycle.Rules, lifecycleRule) - - bucketAttrsToUpdate := googlestorage.BucketAttrsToUpdate{ - Lifecycle: &attrs.Lifecycle, - } - if _, err := bucket.Update(ctx, bucketAttrsToUpdate); err != nil { - return nil, fmt.Errorf("failed to update bucket lifecycle: %w", err) - } - } - return service, nil + return &GoogleCloudStorageService{ + client: client, + bucket: config.Bucket, + objectPrefix: config.ObjectPrefix, + discardAfterTimeout: config.DiscardAfterTimeout, + }, nil } -func (gcs *GoogleCloudStorageService) Put(ctx context.Context, data []byte, expiry uint64) error { - logPut("das.GoogleCloudStorageService.Store", data, expiry, gcs) - if expiry > math.MaxInt64 { - return fmt.Errorf("request expiry time (%v) exceeds max int64", expiry) - } - // #nosec G115 - expiryTime := time.Unix(int64(expiry), 0) - currentTimePlusRetention := time.Now().Add(gcs.maxRetention) - if expiryTime.After(currentTimePlusRetention) { - return fmt.Errorf("requested expiry time (%v) exceeds current time plus maximum allowed retention period(%v)", expiryTime, currentTimePlusRetention) - } - if err := gcs.operator.Upload(ctx, gcs.bucket, gcs.objectPrefix, data); err != nil { - log.Error("das.GoogleCloudStorageService.Store", "err", err) - return err +func buildGoogleCloudStorageClient(accessTokenFile string) (*googlestorage.Client, error) { + // Note that if the credentials are not specified, the client library will find credentials using ADC(Application Default Credentials) + // https://cloud.google.com/docs/authentication/provide-credentials-adc. + if accessTokenFile == "" { + return googlestorage.NewClient(context.Background()) } - return nil + return googlestorage.NewClient(context.Background(), option.WithCredentialsFile(accessTokenFile)) } func (gcs *GoogleCloudStorageService) GetByHash(ctx context.Context, key common.Hash) ([]byte, error) { log.Trace("das.GoogleCloudStorageService.GetByHash", "key", pretty.PrettyHash(key), "this", gcs) - buf, err := gcs.operator.Download(ctx, gcs.bucket, gcs.objectPrefix, key) + obj := gcs.client.Bucket(gcs.bucket).Object(gcs.objectPrefix + EncodeStorageServiceKey(key)) + reader, err := obj.NewReader(ctx) if err != nil { log.Error("das.GoogleCloudStorageService.GetByHash", "err", err) return nil, err } + buf, err := io.ReadAll(reader) + if err != nil { + log.Error("das.GoogleCloudStorageService.GetByHash", "err", err) + } return buf, nil } -func (gcs *GoogleCloudStorageService) ExpirationPolicy(ctx context.Context) (daprovider.ExpirationPolicy, error) { - if gcs.enableExpiry { - return daprovider.KeepForever, nil +func (gcs *GoogleCloudStorageService) Put(ctx context.Context, value []byte, timeout uint64) error { + logPut("das.GoogleCloudStorageService.Store", value, timeout, gcs) + obj := gcs.client.Bucket(gcs.bucket).Object(gcs.objectPrefix + EncodeStorageServiceKey(dastree.Hash(value))) + w := obj.NewWriter(ctx) + if gcs.discardAfterTimeout && timeout <= math.MaxInt64 { + w.Retention = &googlestorage.ObjectRetention{ + Mode: "Unlocked", + RetainUntil: time.Unix(int64(timeout), 0), + } + } + if _, err := fmt.Fprintln(w, value); err != nil { + log.Error("das.GoogleCloudStorageService.Store", "err", err) + return err + } + err := w.Close() + if err != nil { + log.Error("das.GoogleCloudStorageService.Store", "err", err) } - return daprovider.DiscardAfterDataTimeout, nil + return err } func (gcs *GoogleCloudStorageService) Sync(ctx context.Context) error { @@ -173,7 +110,14 @@ func (gcs *GoogleCloudStorageService) Sync(ctx context.Context) error { } func (gcs *GoogleCloudStorageService) Close(ctx context.Context) error { - return gcs.operator.Close(ctx) + return gcs.client.Close() +} + +func (gcs *GoogleCloudStorageService) ExpirationPolicy(ctx context.Context) (daprovider.ExpirationPolicy, error) { + if gcs.discardAfterTimeout { + return daprovider.DiscardAfterDataTimeout, nil + } + return daprovider.KeepForever, nil } func (gcs *GoogleCloudStorageService) String() string { @@ -181,11 +125,9 @@ func (gcs *GoogleCloudStorageService) String() string { } func (gcs *GoogleCloudStorageService) HealthCheck(ctx context.Context) error { - bucket := gcs.operator.Bucket(gcs.bucket) + bucket := gcs.client.Bucket(gcs.bucket) // check if we have bucket permissions permissions := []string{ - "storage.buckets.get", - "storage.buckets.list", "storage.objects.create", "storage.objects.delete", "storage.objects.list", @@ -200,6 +142,5 @@ func (gcs *GoogleCloudStorageService) HealthCheck(ctx context.Context) error { if !cmp.Equal(perms, permissions) { return fmt.Errorf("permissions mismatch (-want +got):\n%s", cmp.Diff(permissions, perms)) } - return nil } From 12a4a053e2bf135872a29e173645b33ec2874874 Mon Sep 17 00:00:00 2001 From: Antonio Nunez Date: Tue, 19 Nov 2024 22:19:00 +0800 Subject: [PATCH 02/10] updated --- das/google_cloud_storage_service.go | 119 ++++++++++++++--------- das/google_cloud_storage_service_test.go | 5 +- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/das/google_cloud_storage_service.go b/das/google_cloud_storage_service.go index adf039347c..85fa796420 100644 --- a/das/google_cloud_storage_service.go +++ b/das/google_cloud_storage_service.go @@ -21,6 +21,51 @@ import ( "github.com/offchainlabs/nitro/util/pretty" ) +type GoogleCloudStorageOperator interface { + Bucket(name string) *googlestorage.BucketHandle + Upload(ctx context.Context, bucket, objectPrefix string, value []byte, discardAfterTimeout bool, timeout uint64) error + Download(ctx context.Context, bucket, objectPrefix string, key common.Hash) ([]byte, error) + Close(ctx context.Context) error +} + +type GoogleCloudStorageClient struct { + client *googlestorage.Client +} + +func (g *GoogleCloudStorageClient) Bucket(name string) *googlestorage.BucketHandle { + return g.client.Bucket(name) +} + +func (g *GoogleCloudStorageClient) Upload(ctx context.Context, bucket, objectPrefix string, value []byte, discardAfterTimeout bool, timeout uint64) error { + obj := g.client.Bucket(bucket).Object(objectPrefix + EncodeStorageServiceKey(dastree.Hash(value))) + w := obj.NewWriter(ctx) + + if discardAfterTimeout && timeout <= math.MaxInt64 { + w.Retention = &googlestorage.ObjectRetention{ + Mode: "Unlocked", + RetainUntil: time.Unix(int64(timeout), 0), + } + } + + if _, err := fmt.Fprintln(w, value); err != nil { + return err + } + return w.Close() +} + +func (g *GoogleCloudStorageClient) Download(ctx context.Context, bucket, objectPrefix string, key common.Hash) ([]byte, error) { + obj := g.client.Bucket(bucket).Object(objectPrefix + EncodeStorageServiceKey(key)) + reader, err := obj.NewReader(ctx) + if err != nil { + return nil, err + } + return io.ReadAll(reader) +} + +func (g *GoogleCloudStorageClient) Close(ctx context.Context) error { + return g.client.Close() +} + type GoogleCloudStorageServiceConfig struct { Enable bool `koanf:"enable"` AccessTokenFile string `koanf:"access-token-file"` @@ -37,72 +82,62 @@ func GoogleCloudConfigAddOptions(prefix string, f *flag.FlagSet) { f.String(prefix+".bucket", DefaultGoogleCloudStorageServiceConfig.Bucket, "Google Cloud Storage bucket") f.String(prefix+".object-prefix", DefaultGoogleCloudStorageServiceConfig.ObjectPrefix, "prefix to add to Google Cloud Storage objects") f.Bool(prefix+".discard-after-timeout", DefaultGoogleCloudStorageServiceConfig.DiscardAfterTimeout, "discard data after its expiry timeout") - } type GoogleCloudStorageService struct { - client *googlestorage.Client + operator GoogleCloudStorageOperator bucket string objectPrefix string discardAfterTimeout bool } func NewGoogleCloudStorageService(config GoogleCloudStorageServiceConfig) (StorageService, error) { - client, err := buildGoogleCloudStorageClient(config.AccessTokenFile) + var client *googlestorage.Client + var err error + // Note that if the credentials are not specified, the client library will find credentials using ADC(Application Default Credentials) + // https://cloud.google.com/docs/authentication/provide-credentials-adc. + if config.AccessTokenFile == "" { + client, err = googlestorage.NewClient(context.Background()) + } else { + client, err = googlestorage.NewClient(context.Background(), option.WithCredentialsFile(config.AccessTokenFile)) + } if err != nil { return nil, fmt.Errorf("error creating Google Cloud Storage client: %w", err) } - return &GoogleCloudStorageService{ - client: client, + service := &GoogleCloudStorageService{ + operator: &GoogleCloudStorageClient{client: client}, bucket: config.Bucket, objectPrefix: config.ObjectPrefix, discardAfterTimeout: config.DiscardAfterTimeout, - }, nil + } + return service, nil } -func buildGoogleCloudStorageClient(accessTokenFile string) (*googlestorage.Client, error) { - // Note that if the credentials are not specified, the client library will find credentials using ADC(Application Default Credentials) - // https://cloud.google.com/docs/authentication/provide-credentials-adc. - if accessTokenFile == "" { - return googlestorage.NewClient(context.Background()) +func (gcs *GoogleCloudStorageService) Put(ctx context.Context, value []byte, timeout uint64) error { + logPut("das.GoogleCloudStorageService.Store", value, timeout, gcs) + + if err := gcs.operator.Upload(ctx, gcs.bucket, gcs.objectPrefix, value, gcs.discardAfterTimeout, timeout); err != nil { + log.Error("das.GoogleCloudStorageService.Store", "err", err) + return err } - return googlestorage.NewClient(context.Background(), option.WithCredentialsFile(accessTokenFile)) + return nil } func (gcs *GoogleCloudStorageService) GetByHash(ctx context.Context, key common.Hash) ([]byte, error) { log.Trace("das.GoogleCloudStorageService.GetByHash", "key", pretty.PrettyHash(key), "this", gcs) - obj := gcs.client.Bucket(gcs.bucket).Object(gcs.objectPrefix + EncodeStorageServiceKey(key)) - reader, err := obj.NewReader(ctx) + buf, err := gcs.operator.Download(ctx, gcs.bucket, gcs.objectPrefix, key) if err != nil { log.Error("das.GoogleCloudStorageService.GetByHash", "err", err) return nil, err } - buf, err := io.ReadAll(reader) - if err != nil { - log.Error("das.GoogleCloudStorageService.GetByHash", "err", err) - } return buf, nil } -func (gcs *GoogleCloudStorageService) Put(ctx context.Context, value []byte, timeout uint64) error { - logPut("das.GoogleCloudStorageService.Store", value, timeout, gcs) - obj := gcs.client.Bucket(gcs.bucket).Object(gcs.objectPrefix + EncodeStorageServiceKey(dastree.Hash(value))) - w := obj.NewWriter(ctx) - if gcs.discardAfterTimeout && timeout <= math.MaxInt64 { - w.Retention = &googlestorage.ObjectRetention{ - Mode: "Unlocked", - RetainUntil: time.Unix(int64(timeout), 0), - } - } - if _, err := fmt.Fprintln(w, value); err != nil { - log.Error("das.GoogleCloudStorageService.Store", "err", err) - return err - } - err := w.Close() - if err != nil { - log.Error("das.GoogleCloudStorageService.Store", "err", err) +func (gcs *GoogleCloudStorageService) ExpirationPolicy(ctx context.Context) (daprovider.ExpirationPolicy, error) { + if gcs.discardAfterTimeout { + return daprovider.DiscardAfterDataTimeout, nil } - return err + return daprovider.KeepForever, nil } func (gcs *GoogleCloudStorageService) Sync(ctx context.Context) error { @@ -110,14 +145,7 @@ func (gcs *GoogleCloudStorageService) Sync(ctx context.Context) error { } func (gcs *GoogleCloudStorageService) Close(ctx context.Context) error { - return gcs.client.Close() -} - -func (gcs *GoogleCloudStorageService) ExpirationPolicy(ctx context.Context) (daprovider.ExpirationPolicy, error) { - if gcs.discardAfterTimeout { - return daprovider.DiscardAfterDataTimeout, nil - } - return daprovider.KeepForever, nil + return gcs.operator.Close(ctx) } func (gcs *GoogleCloudStorageService) String() string { @@ -125,7 +153,7 @@ func (gcs *GoogleCloudStorageService) String() string { } func (gcs *GoogleCloudStorageService) HealthCheck(ctx context.Context) error { - bucket := gcs.client.Bucket(gcs.bucket) + bucket := gcs.operator.Bucket(gcs.bucket) // check if we have bucket permissions permissions := []string{ "storage.objects.create", @@ -142,5 +170,6 @@ func (gcs *GoogleCloudStorageService) HealthCheck(ctx context.Context) error { if !cmp.Equal(perms, permissions) { return fmt.Errorf("permissions mismatch (-want +got):\n%s", cmp.Diff(permissions, perms)) } + return nil } diff --git a/das/google_cloud_storage_service_test.go b/das/google_cloud_storage_service_test.go index 94d6f3ee44..545437708b 100644 --- a/das/google_cloud_storage_service_test.go +++ b/das/google_cloud_storage_service_test.go @@ -34,7 +34,7 @@ func (c *mockGCSClient) Close(ctx context.Context) error { return nil } -func (c *mockGCSClient) Upload(ctx context.Context, bucket, objectPrefix string, value []byte) error { +func (c *mockGCSClient) Upload(ctx context.Context, bucket, objectPrefix string, value []byte, discardAfterTimeout bool, timeout uint64) error { key := objectPrefix + EncodeStorageServiceKey(dastree.Hash(value)) c.storage[key] = value return nil @@ -47,7 +47,7 @@ func NewTestGoogleCloudStorageService(ctx context.Context, googleCloudStorageCon operator: &mockGCSClient{ storage: make(map[string][]byte), }, - maxRetention: googleCloudStorageConfig.MaxRetention, + discardAfterTimeout: true, }, nil } @@ -57,7 +57,6 @@ func TestNewGoogleCloudStorageService(t *testing.T) { expiry := uint64(time.Now().Add(time.Hour).Unix()) googleCloudStorageServiceConfig := DefaultGoogleCloudStorageServiceConfig googleCloudStorageServiceConfig.Enable = true - googleCloudStorageServiceConfig.MaxRetention = time.Hour * 24 googleCloudService, err := NewTestGoogleCloudStorageService(ctx, googleCloudStorageServiceConfig) Require(t, err) From 26cfce8ff986633bd8ebd5394d7a3f789ca85472 Mon Sep 17 00:00:00 2001 From: Antonio Nunez Date: Wed, 20 Nov 2024 13:58:41 +0800 Subject: [PATCH 03/10] support both access-token and access-token-file parameters --- das/google_cloud_storage_service.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/das/google_cloud_storage_service.go b/das/google_cloud_storage_service.go index 85fa796420..71924a690c 100644 --- a/das/google_cloud_storage_service.go +++ b/das/google_cloud_storage_service.go @@ -68,6 +68,7 @@ func (g *GoogleCloudStorageClient) Close(ctx context.Context) error { type GoogleCloudStorageServiceConfig struct { Enable bool `koanf:"enable"` + AccessToken string `koanf:"access-token"` AccessTokenFile string `koanf:"access-token-file"` Bucket string `koanf:"bucket"` ObjectPrefix string `koanf:"object-prefix"` @@ -78,7 +79,8 @@ var DefaultGoogleCloudStorageServiceConfig = GoogleCloudStorageServiceConfig{} func GoogleCloudConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".enable", DefaultGoogleCloudStorageServiceConfig.Enable, "EXPERIMENTAL/unsupported - enable storage/retrieval of sequencer batch data from an Google Cloud Storage bucket") - f.String(prefix+".access-token-file", DefaultGoogleCloudStorageServiceConfig.AccessTokenFile, "Google Cloud Storage access token") + f.String(prefix+".access-token", DefaultGoogleCloudStorageServiceConfig.AccessToken, "Google Cloud Storage access token (JSON string)") + f.String(prefix+".access-token-file", DefaultGoogleCloudStorageServiceConfig.AccessTokenFile, "Google Cloud Storage access token (JSON file path)") f.String(prefix+".bucket", DefaultGoogleCloudStorageServiceConfig.Bucket, "Google Cloud Storage bucket") f.String(prefix+".object-prefix", DefaultGoogleCloudStorageServiceConfig.ObjectPrefix, "prefix to add to Google Cloud Storage objects") f.Bool(prefix+".discard-after-timeout", DefaultGoogleCloudStorageServiceConfig.DiscardAfterTimeout, "discard data after its expiry timeout") @@ -96,10 +98,12 @@ func NewGoogleCloudStorageService(config GoogleCloudStorageServiceConfig) (Stora var err error // Note that if the credentials are not specified, the client library will find credentials using ADC(Application Default Credentials) // https://cloud.google.com/docs/authentication/provide-credentials-adc. - if config.AccessTokenFile == "" { - client, err = googlestorage.NewClient(context.Background()) - } else { + if config.AccessToken != "" { + client, err = googlestorage.NewClient(context.Background(), option.WithCredentialsJSON([]byte(config.AccessToken))) + } else if config.AccessTokenFile != "" { client, err = googlestorage.NewClient(context.Background(), option.WithCredentialsFile(config.AccessTokenFile)) + } else { + client, err = googlestorage.NewClient(context.Background()) } if err != nil { return nil, fmt.Errorf("error creating Google Cloud Storage client: %w", err) From a471aab128545224dfcad0431cf10614335f193c Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Fri, 29 Nov 2024 14:27:03 +0100 Subject: [PATCH 04/10] Create ADR-0001 Avoid primative constraint types This PR creates the ADR (Architectural Decision Record) structure in the docs in our repository and adds the first two ADRs. The first is sort of a meta-ADR in that it just captures our decision to actually use ADRs to record our architecturally-significant decisions for posterity and which format to use. --- ...markdown-architectural-decision-records.md | 29 ++++++ .../0001-avoid-primative-constraint-types.md | 96 +++++++++++++++++++ docs/decisions/README.md | 10 ++ docs/decisions/adr-template-bare-minimal.md | 16 ++++ docs/decisions/adr-template-bare.md | 44 +++++++++ docs/decisions/adr-template-minimal.md | 23 +++++ docs/decisions/adr-template.md | 74 ++++++++++++++ 7 files changed, 292 insertions(+) create mode 100644 docs/decisions/0000-use-markdown-architectural-decision-records.md create mode 100644 docs/decisions/0001-avoid-primative-constraint-types.md create mode 100644 docs/decisions/README.md create mode 100644 docs/decisions/adr-template-bare-minimal.md create mode 100644 docs/decisions/adr-template-bare.md create mode 100644 docs/decisions/adr-template-minimal.md create mode 100644 docs/decisions/adr-template.md diff --git a/docs/decisions/0000-use-markdown-architectural-decision-records.md b/docs/decisions/0000-use-markdown-architectural-decision-records.md new file mode 100644 index 0000000000..506f5fa28b --- /dev/null +++ b/docs/decisions/0000-use-markdown-architectural-decision-records.md @@ -0,0 +1,29 @@ +# Use Markdown Architectural Decision Records + +## Context and Problem Statement + +We want to record architectural decisions made in this project independent +whether decisions concern the architecture ("architectural decision record"), +the code, or other fields. + +Which format and structure should these records follow? + +## Considered Options + +* [MADR](https://adr.github.io/madr/) 4.0.0 – The Markdown Architectural Decision Records +* [Michael Nygard's template](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions) – The first incarnation of the term "ADR" +* [Sustainable Architectural Decisions](https://www.infoq.com/articles/sustainable-architectural-design-decisions) – The Y-Statements +* Other templates listed at +* Formless – No conventions for file format and structure + +## Decision Outcome + +Chosen option: "MADR 4.0.0", because + +* Implicit assumptions should be made explicit. + Design documentation is important to enable people understanding the decisions later on. + See also ["A rational design process: How and why to fake it"](https://doi.org/10.1109/TSE.1986.6312940). +* MADR allows for structured capturing of any decision. +* The MADR format is lean and fits our development style. +* The MADR structure is comprehensible and facilitates usage & maintenance. +* The MADR project is vivid. diff --git a/docs/decisions/0001-avoid-primative-constraint-types.md b/docs/decisions/0001-avoid-primative-constraint-types.md new file mode 100644 index 0000000000..b52cea8bdd --- /dev/null +++ b/docs/decisions/0001-avoid-primative-constraint-types.md @@ -0,0 +1,96 @@ +--- +status: accepted +date: 2024-11-29 +decision-makers: eljobe@ plasmapower@ +--- + +# Avoid primative constraint types + +## Context and Problem Statement + +When working on the go code for BoLD, we became slightly annoyed that several +places in the history package were checking the constraint that the `virtual` +argumet to a function was positive. One possible workaround would have been +to create a constrained wrapper type around `uint64` which would only allow +positive values. For example: + +```go +// Pos64 is a type which represents a positive uint64. +// +// The "zero" value of Pos64 is 1. +type Pos64 struct { + uint64 +} + +// NewPos64 returns a new Pos64 with the given value. +// +// errors if v is 0. +func NewPos64(v uint64) (Pos64, error) { + if v == 0 { + return Pos64{}, errors.New("v must be positive. got: 0") + } + return Pos64{v}, nil +} + +// MustPos64 returns a new Pos64 with the given value. +// +// panics if v is 0. +func MustPos64(v uint64) Pos64 { + if v == 0 { + panic("v must be positive. got: 0") + } + return Pos64{v} +} + +// Val returns the value of the Pos64. +func (p Pos64) Val() uint64 { + // The zero value of Pos64 is 1. + if p.uint64 == 0 { + return 1 + } + return p.uint64 +} +``` + +The idea being that within a package, all of the functions which needed to deal +with a `virtual` argument, could take in a `Pos64` instead of a `uint64` and it +would be up to clients of the package to ensure that they only passed in +positive values. + +## Considered Options + +* New Package: `util/chk` for checking type constraint +* Status Quo: check the constraint in multiple places +* Minimize Checks: no check in package private functions + +## Decision Outcome + +Chosen option: "Status Quo", because the "New Package" option introduces a +regression in being able to use type type with operators, and "Minimize Checks" +is too prone to bugs introduced by refactoring. + + +## Pros and Cons of the Options + +### New Pacakge: `util/chk` for checking type constraint + +* Good, because it is expressive +* Good, because the constraint only needs to be checked during construction +* Bad, because `Pos64` doesn't compile with operators like `+ * - /` + +### Status Quo: check the constraint in multiple places + +* Good, because it is what the code is already doing +* Good, because when a funciton becomes public, the constraint holds +* Good, because when a function moves to another file or package, the constraint holds +* Bad, because it means the check may need to be repeated. DRY + +### Minimize Checks: no check in package private functions + +* Good, because it reduces the amount of times a constraint is checked +* Bad, because the assumption might be violated if a private function becomes + public, or gains an additional caller. + +## More Information + +See the discussion on now-closed #2743 diff --git a/docs/decisions/README.md b/docs/decisions/README.md new file mode 100644 index 0000000000..63b1a18cd2 --- /dev/null +++ b/docs/decisions/README.md @@ -0,0 +1,10 @@ +# Decisions + +For new Architectural Decision Records (ADRs), please use one of the following templates as a starting point: + +* [adr-template.md](adr-template.md) has all sections, with explanations about them. +* [adr-template-minmal.md](adr-template-minimal.md) only contains mandatory sections, with explanations about them. +* [adr-template-bare.md](adr-template-bare.md) has all sections, wich are empty (no explanations). +* [adr-template-bare-minimal.md](adr-template-bare-minimal.md) has the mandatory sections, without explanations. + +The MADR documentation is available at while general information about ADRs is available at . diff --git a/docs/decisions/adr-template-bare-minimal.md b/docs/decisions/adr-template-bare-minimal.md new file mode 100644 index 0000000000..bd16d0ea21 --- /dev/null +++ b/docs/decisions/adr-template-bare-minimal.md @@ -0,0 +1,16 @@ +# + +## Context and Problem Statement + + + +## Considered Options + + + +## Decision Outcome + + + +### Consequences + diff --git a/docs/decisions/adr-template-bare.md b/docs/decisions/adr-template-bare.md new file mode 100644 index 0000000000..26f6598f70 --- /dev/null +++ b/docs/decisions/adr-template-bare.md @@ -0,0 +1,44 @@ +--- +status: +date: +decision-makers: +consulted: +informed: +--- + +# + +## Context and Problem Statement + + + +## Decision Drivers + +* + +## Considered Options + +* + +## Decision Outcome + +Chosen option: "", because + +### Consequences + +* Good, because +* Bad, because + +### Confirmation + + + +## Pros and Cons of the Options + +### + +* Good, because +* Neutral, because +* Bad, because + +## More Information diff --git a/docs/decisions/adr-template-minimal.md b/docs/decisions/adr-template-minimal.md new file mode 100644 index 0000000000..267640bfbc --- /dev/null +++ b/docs/decisions/adr-template-minimal.md @@ -0,0 +1,23 @@ +# {short title, representative of solved problem and found solution} + +## Context and Problem Statement + +{Describe the context and problem statement, e.g., in free form using two to three sentences or in the form of an illustrative story. You may want to articulate the problem in form of a question and add links to collaboration boards or issue management systems.} + +## Considered Options + +* {title of option 1} +* {title of option 2} +* {title of option 3} +* … + +## Decision Outcome + +Chosen option: "{title of option 1}", because {justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force {force} | … | comes out best (see below)}. + + +### Consequences + +* Good, because {positive consequence, e.g., improvement of one or more desired qualities, …} +* Bad, because {negative consequence, e.g., compromising one or more desired qualities, …} +* … diff --git a/docs/decisions/adr-template.md b/docs/decisions/adr-template.md new file mode 100644 index 0000000000..08dac30ed8 --- /dev/null +++ b/docs/decisions/adr-template.md @@ -0,0 +1,74 @@ +--- +# These are optional metadata elements. Feel free to remove any of them. +status: "{proposed | rejected | accepted | deprecated | … | superseded by ADR-0123" +date: {YYYY-MM-DD when the decision was last updated} +decision-makers: {list everyone involved in the decision} +consulted: {list everyone whose opinions are sought (typically subject-matter experts); and with whom there is a two-way communication} +informed: {list everyone who is kept up-to-date on progress; and with whom there is a one-way communication} +--- + +# {short title, representative of solved problem and found solution} + +## Context and Problem Statement + +{Describe the context and problem statement, e.g., in free form using two to three sentences or in the form of an illustrative story. You may want to articulate the problem in form of a question and add links to collaboration boards or issue management systems.} + + +## Decision Drivers + +* {decision driver 1, e.g., a force, facing concern, …} +* {decision driver 2, e.g., a force, facing concern, …} +* … + +## Considered Options + +* {title of option 1} +* {title of option 2} +* {title of option 3} +* … + +## Decision Outcome + +Chosen option: "{title of option 1}", because {justification. e.g., only option, which meets k.o. criterion decision driver | which resolves force {force} | … | comes out best (see below)}. + + +### Consequences + +* Good, because {positive consequence, e.g., improvement of one or more desired qualities, …} +* Bad, because {negative consequence, e.g., compromising one or more desired qualities, …} +* … + + +### Confirmation + +{Describe how the implementation of/compliance with the ADR can/will be confirmed. Are the design that was decided for and its implementation in line with the decision made? E.g., a design/code review or a test with a library such as ArchUnit can help validate this. Not that although we classify this element as optional, it is included in many ADRs.} + + +## Pros and Cons of the Options + +### {title of option 1} + + +{example | description | pointer to more information | …} + +* Good, because {argument a} +* Good, because {argument b} + +* Neutral, because {argument c} +* Bad, because {argument d} +* … + +### {title of other option} + +{example | description | pointer to more information | …} + +* Good, because {argument a} +* Good, because {argument b} +* Neutral, because {argument c} +* Bad, because {argument d} +* … + + +## More Information + +{You might want to provide additional evidence/confidence for the decision outcome here and/or document the team agreement on the decision and/or define when/how this decision the decision should be realized and if/when it should be re-visited. Links to other decisions and resources might appear here as well.} From f3139eaa1ed6b605aea38a4aa844e38ddd8976ae Mon Sep 17 00:00:00 2001 From: Joshua Colvin Date: Sat, 11 Jan 2025 20:37:16 -0700 Subject: [PATCH 05/10] Fix `--execution.rpc.gas-cap=0` to again mean infinite gas There was a regression introduced in v3.3.0 where specifying `--execution.rpc.gas-cap=0` would cause gas related errors. --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index dd32b782ed..aaded574dc 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit dd32b782ed255c1ac20ed5beee11dd6a821f9be2 +Subproject commit aaded574dcf7b2f6b5851ee858603cb61335217e From 26b4c1028fdc8475c349510dbe392d99e27dc082 Mon Sep 17 00:00:00 2001 From: dashangcun <907225865@qq.com> Date: Mon, 13 Jan 2025 17:42:03 +0800 Subject: [PATCH 06/10] refactor: using slices.Contains to simplify the code Signed-off-by: dashangcun <907225865@qq.com> --- cmd/conf/init.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cmd/conf/init.go b/cmd/conf/init.go index 74bd89fd16..97b31958d5 100644 --- a/cmd/conf/init.go +++ b/cmd/conf/init.go @@ -3,6 +3,7 @@ package conf import ( "fmt" "runtime" + "slices" "strings" "time" @@ -106,7 +107,7 @@ func (c *InitConfig) Validate() error { if c.Force && c.RecreateMissingStateFrom > 0 { log.Warn("force init enabled, recreate-missing-state-from will have no effect") } - if c.Latest != "" && !isAcceptedSnapshotKind(c.Latest) { + if c.Latest != "" && !slices.Contains(acceptedSnapshotKinds, c.Latest) { return fmt.Errorf("invalid value for latest option: \"%s\" %s", c.Latest, acceptedSnapshotKindsStr) } if c.Prune != "" && c.PruneThreads <= 0 { @@ -139,12 +140,3 @@ var ( acceptedSnapshotKinds = []string{"archive", "pruned", "genesis"} acceptedSnapshotKindsStr = "(accepted values: \"" + strings.Join(acceptedSnapshotKinds, "\" | \"") + "\")" ) - -func isAcceptedSnapshotKind(kind string) bool { - for _, valid := range acceptedSnapshotKinds { - if kind == valid { - return true - } - } - return false -} From c721fdbf0f6e824b25f2c78875b079bb0d2aa5ed Mon Sep 17 00:00:00 2001 From: Pepper Lebeck-Jobe Date: Mon, 13 Jan 2025 18:43:37 +0100 Subject: [PATCH 07/10] Fix typo pimative -> primitive --- ...traint-types.md => 0001-avoid-primitive-constraint-types.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/decisions/{0001-avoid-primative-constraint-types.md => 0001-avoid-primitive-constraint-types.md} (98%) diff --git a/docs/decisions/0001-avoid-primative-constraint-types.md b/docs/decisions/0001-avoid-primitive-constraint-types.md similarity index 98% rename from docs/decisions/0001-avoid-primative-constraint-types.md rename to docs/decisions/0001-avoid-primitive-constraint-types.md index b52cea8bdd..8a3ecb0632 100644 --- a/docs/decisions/0001-avoid-primative-constraint-types.md +++ b/docs/decisions/0001-avoid-primitive-constraint-types.md @@ -4,7 +4,7 @@ date: 2024-11-29 decision-makers: eljobe@ plasmapower@ --- -# Avoid primative constraint types +# Avoid primitive constraint types ## Context and Problem Statement From b7e28f2f12c8b808ade3f0103d45d780cdb19c83 Mon Sep 17 00:00:00 2001 From: Joshua Colvin Date: Mon, 13 Jan 2025 11:53:45 -0700 Subject: [PATCH 08/10] Update geth pin to pull in extra comment --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index aaded574dc..779b669ac0 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit aaded574dcf7b2f6b5851ee858603cb61335217e +Subproject commit 779b669ac0d0020099a67a1c39fbaf66b901c1a5 From 84bfafa45edf97a50828a0d57790cdaed3999b42 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 14 Jan 2025 10:25:11 -0700 Subject: [PATCH 09/10] Enable the geth trie prefetcher for the sequencer --- execution/gethexec/executionengine.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/execution/gethexec/executionengine.go b/execution/gethexec/executionengine.go index 7cf83c7183..e606027419 100644 --- a/execution/gethexec/executionengine.go +++ b/execution/gethexec/executionengine.go @@ -508,6 +508,8 @@ func (s *ExecutionEngine) sequenceTransactionsWithBlockMutex(header *arbostypes. if err != nil { return nil, err } + statedb.StartPrefetcher("Sequencer") + defer statedb.StopPrefetcher() delayedMessagesRead := lastBlockHeader.Nonce.Uint64() From b18e8f0d8d48093c7a2986404ae78870b76c888b Mon Sep 17 00:00:00 2001 From: Joshua Colvin Date: Tue, 14 Jan 2025 14:39:00 -0700 Subject: [PATCH 10/10] Revert wasmer update New version of wasmer considers binaries created by current version of wasmer as incompatible. Will delay merging in new versions to reduce the number of times that stylus contracts will need to be recompiled. This reverts commit 93841cee1db6d2fc826c169992aa8bb73163703b, reversing changes made to 3ba8fc2dd39315e2d449f5f6666c23a472d11cf1. --- arbitrator/Cargo.lock | 61 +++++++++++----------------- arbitrator/tools/wasmer | 2 +- arbitrator/wasm-libraries/Cargo.lock | 14 +------ 3 files changed, 25 insertions(+), 52 deletions(-) diff --git a/arbitrator/Cargo.lock b/arbitrator/Cargo.lock index 9688d07229..2b437968fa 100644 --- a/arbitrator/Cargo.lock +++ b/arbitrator/Cargo.lock @@ -747,12 +747,11 @@ dependencies = [ [[package]] name = "dashmap" -version = "6.1.0" +version = "5.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" +checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils", "hashbrown 0.14.5", "lock_api", "once_cell", @@ -975,10 +974,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" dependencies = [ "cfg-if 1.0.0", - "js-sys", "libc", "wasi", - "wasm-bindgen", ] [[package]] @@ -1315,6 +1312,15 @@ dependencies = [ "hashbrown 0.14.5", ] +[[package]] +name = "mach" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b823e83b2affd8f40a9ee8c29dbc56404c1e34cd2710921f2801e2cf29527afa" +dependencies = [ + "libc", +] + [[package]] name = "mach2" version = "0.4.2" @@ -2552,7 +2558,7 @@ dependencies = [ [[package]] name = "wasmer" -version = "4.3.7" +version = "4.2.8" dependencies = [ "bytes", "cfg-if 1.0.0", @@ -2574,12 +2580,12 @@ dependencies = [ "wasmer-types", "wasmer-vm", "wat", - "windows-sys 0.59.0", + "winapi", ] [[package]] name = "wasmer-compiler" -version = "4.3.7" +version = "4.2.8" dependencies = [ "backtrace", "bytes", @@ -2588,7 +2594,6 @@ dependencies = [ "enumset", "lazy_static", "leb128", - "libc", "memmap2 0.5.10", "more-asserts", "region", @@ -2600,13 +2605,12 @@ dependencies = [ "wasmer-types", "wasmer-vm", "wasmparser", - "windows-sys 0.59.0", - "xxhash-rust", + "winapi", ] [[package]] name = "wasmer-compiler-cranelift" -version = "4.3.7" +version = "4.2.8" dependencies = [ "cranelift-codegen", "cranelift-entity", @@ -2623,7 +2627,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-llvm" -version = "4.3.7" +version = "4.2.8" dependencies = [ "byteorder", "cc", @@ -2645,7 +2649,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-singlepass" -version = "4.3.7" +version = "4.2.8" dependencies = [ "byteorder", "dynasm", @@ -2662,7 +2666,7 @@ dependencies = [ [[package]] name = "wasmer-derive" -version = "4.3.7" +version = "4.2.8" dependencies = [ "proc-macro-error", "proc-macro2", @@ -2672,25 +2676,21 @@ dependencies = [ [[package]] name = "wasmer-types" -version = "4.3.7" +version = "4.2.8" dependencies = [ "bytecheck", "enum-iterator 0.7.0", "enumset", - "getrandom", - "hex", "indexmap 1.9.3", "more-asserts", "rkyv", - "sha2 0.10.8", "target-lexicon", "thiserror", - "xxhash-rust", ] [[package]] name = "wasmer-vm" -version = "4.3.7" +version = "4.2.8" dependencies = [ "backtrace", "cc", @@ -2704,14 +2704,14 @@ dependencies = [ "indexmap 1.9.3", "lazy_static", "libc", - "mach2", + "mach", "memoffset", "more-asserts", "region", "scopeguard", "thiserror", "wasmer-types", - "windows-sys 0.59.0", + "winapi", ] [[package]] @@ -2830,15 +2830,6 @@ dependencies = [ "windows-targets", ] -[[package]] -name = "windows-sys" -version = "0.59.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" -dependencies = [ - "windows-targets", -] - [[package]] name = "windows-targets" version = "0.52.6" @@ -2951,12 +2942,6 @@ dependencies = [ "tap", ] -[[package]] -name = "xxhash-rust" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a5cbf750400958819fb6178eaa83bee5cd9c29a26a40cc241df8c70fdd46984" - [[package]] name = "zerocopy" version = "0.6.6" diff --git a/arbitrator/tools/wasmer b/arbitrator/tools/wasmer index 84aec79c13..6b15433d83 160000 --- a/arbitrator/tools/wasmer +++ b/arbitrator/tools/wasmer @@ -1 +1 @@ -Subproject commit 84aec79c13888bf3fb324ddbd69b3fecc22d4a8c +Subproject commit 6b15433d83f951555c24f0c56dc05e4751b0cc76 diff --git a/arbitrator/wasm-libraries/Cargo.lock b/arbitrator/wasm-libraries/Cargo.lock index e62acf43a6..a5a066e5c9 100644 --- a/arbitrator/wasm-libraries/Cargo.lock +++ b/arbitrator/wasm-libraries/Cargo.lock @@ -518,10 +518,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" dependencies = [ "cfg-if 1.0.0", - "js-sys", "libc", "wasi", - "wasm-bindgen", ] [[package]] @@ -1635,20 +1633,16 @@ dependencies = [ [[package]] name = "wasmer-types" -version = "4.3.7" +version = "4.2.8" dependencies = [ "bytecheck", "enum-iterator 0.7.0", "enumset", - "getrandom", - "hex", "indexmap 1.9.3", "more-asserts", "rkyv", - "sha2 0.10.8", "target-lexicon", "thiserror", - "xxhash-rust", ] [[package]] @@ -1809,12 +1803,6 @@ dependencies = [ "tap", ] -[[package]] -name = "xxhash-rust" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a5cbf750400958819fb6178eaa83bee5cd9c29a26a40cc241df8c70fdd46984" - [[package]] name = "zerocopy" version = "0.7.35"