From 2e6ce8bc801bb71b7b215d712af0ff0dbd45234c Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 09:06:24 -0700 Subject: [PATCH 1/9] feat(registry): add the contract registry package and chain file format A contract scenario deploys its contract at the start of every run, so a run against a long-lived chain leaves another copy behind and every canary restart moves its contract to a new address. Nothing in the repo can say "this contract already exists on this chain." This adds the leaf package that will answer that. It holds the types, the chain file format, and the loader. It resolves nothing and verifies nothing yet; PLT-1056 is the first caller that reads a chain. The import boundary comes first, deliberately. boundary_test.go asserts that registry imports no sei-load package except generator/bindings, and it went in before any type existed so it guards every later task. It shells out to `go list -deps` rather than walking imports by hand, so a package this one imports cannot smuggle a forbidden dependency in behind it. Verified by adding a config import and watching it fail. Two notes on what landed differently from the plan: chains/ carries a README rather than a .gitkeep. `//go:embed chains` cannot compile against a directory holding only a dotfile, because embed excludes them. The README satisfies the pattern and tells an operator what belongs there, which the dotfile did not. Load validates rather than trusting the file. A misspelled key would leave a field at its zero value, and a contract at the zero address holds no code on any chain, so the run would send load that does nothing. The strict decode mirrors config.decodeStrict: unknown fields and trailing data both fail. Closes CDR-005, CDR-014, CDR-015, CDR-017, CDR-018, CDR-019, CDR-020. Verifiers: gofmt clean, go vet clean, staticcheck clean, golangci-lint 0 issues, go test ./registry/ 8 tests passing. `make verify` reaches check-bindings and stops there: the Makefile fetches solc-static-linux and this host is Darwin, so that step is CI-only. No .sol file or binding changed. Co-Authored-By: Claude Opus 5 (1M context) --- registry/boundary_test.go | 45 +++++ registry/chains/README.md | 24 +++ registry/doc.go | 58 ++++++ registry/registry.go | 204 +++++++++++++++++++++ registry/registry_test.go | 256 +++++++++++++++++++++++++++ registry/testdata/README.md | 11 ++ registry/testdata/no-contract.json | 6 + registry/testdata/with-contract.json | 13 ++ 8 files changed, 617 insertions(+) create mode 100644 registry/boundary_test.go create mode 100644 registry/chains/README.md create mode 100644 registry/doc.go create mode 100644 registry/registry.go create mode 100644 registry/registry_test.go create mode 100644 registry/testdata/README.md create mode 100644 registry/testdata/no-contract.json create mode 100644 registry/testdata/with-contract.json diff --git a/registry/boundary_test.go b/registry/boundary_test.go new file mode 100644 index 0000000..ed3b966 --- /dev/null +++ b/registry/boundary_test.go @@ -0,0 +1,45 @@ +package registry_test + +import ( + "os/exec" + "strings" + "testing" +) + +// modulePath is this repo's module path. A dependency starting with it is a +// sei-load package; anything else is the standard library or a third party. +const modulePath = "github.com/sei-protocol/sei-load" + +// allowedInternalDeps are the sei-load packages the registry may import. +// +// generator/bindings holds the ABI and the bytecode the registry hashes against. +// Nothing else belongs here: reaching for config or types would tie the registry +// to the load generator, and CDR-017 exists to stop that. +var allowedInternalDeps = map[string]bool{ + modulePath + "/generator/bindings": true, +} + +// TestImportBoundary asserts CDR-017: the registry imports no sei-load package +// except the contract bindings. +// +// It shells out to go list rather than walking imports by hand, because -deps +// reports the transitive set. A package this one imports cannot smuggle a +// forbidden dependency in behind it. +func TestImportBoundary(t *testing.T) { + out, err := exec.Command("go", "list", "-deps", ".").Output() + if err != nil { + t.Fatalf("go list -deps .: %v", err) + } + + for _, dep := range strings.Fields(string(out)) { + if !strings.HasPrefix(dep, modulePath) { + continue + } + if dep == modulePath+"/registry" || allowedInternalDeps[dep] { + continue + } + t.Errorf("registry imports %s, which CDR-017 forbids.\n"+ + "The registry may import only the contract bindings. Depending on "+ + "another sei-load package ties it to the load generator.", dep) + } +} diff --git a/registry/chains/README.md b/registry/chains/README.md new file mode 100644 index 0000000..882a409 --- /dev/null +++ b/registry/chains/README.md @@ -0,0 +1,24 @@ +# Compiled-in chain files + +One JSON file per long-lived chain, named for the chain: `arctic-1.json`. +Everything here compiles into the binary, so a run against one of these chains +needs no file supplied at deploy time. + +This directory is empty of chain files today. No contracts exist on arctic-1, +atlantic-2 or pacific-1 yet, and an entry naming a contract that is not there +fails every run that reads it. + +## Adding a chain + +A run against a chain with no entry deploys its contracts and writes a chain +file next to its run report. Review that file and commit it here. Do not write +one by hand: the code hash has to be the hash the chain actually serves, and a +run observes it rather than guessing. + +`WriteChain` refuses to write into this directory. Pointing a run at the +committed registry does not let it edit the committed registry. + +## The format is frozen + +Adding a field is fine. Renaming or repurposing one is a migration, because +every committed file has to change with it. See `registry/doc.go`. diff --git a/registry/doc.go b/registry/doc.go new file mode 100644 index 0000000..a0bd4e6 --- /dev/null +++ b/registry/doc.go @@ -0,0 +1,58 @@ +// Package registry answers one question: does this contract already exist on +// this chain? It looks a contract up by chain identity, verifies that the code +// at the recorded address is the code that was recorded, and produces the entry +// to write after a fresh deployment. +// +// It does not deploy, and it does not bind. A preparation step outside this +// package calls Resolve for each contract a profile needs, deploys where the +// registry has nothing, binds every result with one client, and hands each +// scenario its bound contract. A scenario therefore never holds an address. +// +// # Import boundary +// +// This package imports no other sei-load package except generator/bindings. +// boundary_test.go asserts that. The rule is what keeps this package +// extractable: reaching for config or types would tie the registry to the load +// generator it exists to stay independent of. +// +// The one chain call this package makes is CodeReader.CodeAt. A caller supplies +// an *ethclient.Client, or a test supplies a fake, so this package imports no +// Ethereum client either. +// +// # Chain identity +// +// A chain is identified by its EVM chain id and its genesis hash together, and +// by nothing else. An EVM chain id alone does not identify a chain instance: a +// devnet keeps its id across a re-genesis, so an entry recorded before the +// re-genesis names an address that no longer holds its contract. +// +// chainName and genesisS3URI ride along for a human reading a failure. Neither +// is ever matched on. +// +// # The chain file (FROZEN one-way door) +// +// One file per chain. Files the binary carries live in chains/ and compile in. +// A deployment supplies an extra file by path. +// +// { +// "chainId": 713715, +// "chainName": "arctic-1", +// "genesisHash": "3f1a...", +// "genesisS3URI": "s3://prod-sei-k8s-genesis-artifacts/arctic-1/genesis.json", +// "contracts": [ +// { +// "name": "storagerw", +// "address": "0x1234567890123456789012345678901234567890", +// "codeHash": "0xabcd..." +// } +// ] +// } +// +// Once a committed chain file exists, changing this shape is a migration. Add a +// field, never rename or repurpose one. +// +// Two hashes, two algorithms. codeHash is Keccak-256, because the EVM already +// defines an account's code hash that way, so a reader can check the value +// against chain state rather than only against eth_getCode. genesisHash stays +// SHA-256, because the controller defines it and this file does not redefine it. +package registry diff --git a/registry/registry.go b/registry/registry.go new file mode 100644 index 0000000..9d72083 --- /dev/null +++ b/registry/registry.go @@ -0,0 +1,204 @@ +package registry + +import ( + "bytes" + "embed" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + + "github.com/ethereum/go-ethereum/common" +) + +// embeddedChains holds every chain file the binary carries. The directory ships +// empty: an entry belongs here only once its contracts exist on a long-lived +// chain, and an entry naming a contract that is not there fails every run that +// reads it. +// +//go:embed chains +var embeddedChains embed.FS + +// embeddedSource is the source name Sources reports for a compiled-in chain. +const embeddedSource = "embedded" + +// Contract is one named contract on one chain. A scenario is a list of these, +// and a list of one is ordinary. +type Contract struct { + Name string `json:"name"` + Address common.Address `json:"address"` + // CodeHash is Keccak-256 of the runtime code, observed at deployment. See + // the package doc for why this is Keccak-256 while GenesisHash is SHA-256. + CodeHash common.Hash `json:"codeHash"` +} + +// Chain is one chain and the contracts deployed on it. +type Chain struct { + ChainID int64 `json:"chainId"` + ChainName string `json:"chainName"` + // GenesisHash is bare hex with no algorithm prefix, matching + // SeiNetwork.Status.GenesisHash. + GenesisHash string `json:"genesisHash"` + // GenesisS3URI records where the genesis came from. The registry never + // resolves it, which keeps S3 credentials out of a load generator. + GenesisS3URI string `json:"genesisS3URI,omitempty"` + Contracts []Contract `json:"contracts"` +} + +// Contract returns one named contract. It reports false when the chain carries +// no entry for that name, which is the deploy case rather than a failure. +func (c Chain) Contract(name string) (Contract, bool) { + for _, contract := range c.Contracts { + if contract.Name == name { + return contract, true + } + } + return Contract{}, false +} + +// chainKey is the whole of a chain's identity. An EVM chain id alone does not +// identify a chain instance, because a devnet keeps its id across a re-genesis. +type chainKey struct { + chainID int64 + genesisHash string +} + +// Registry holds every chain the binary carries, plus any the deployment +// supplied. +type Registry struct { + chains map[chainKey]Chain + sources map[chainKey]string +} + +// Load returns the registry the binary carries, with any supplied files layered +// over it in the order given. A supplied file naming a chain the binary carries +// replaces that chain's entry. +// +// Load with no paths returns the embedded registry alone. That is the +// long-lived chain case, and it touches no disk and no network. +func Load(paths ...string) (*Registry, error) { + r := &Registry{ + chains: make(map[chainKey]Chain), + sources: make(map[chainKey]string), + } + + if err := r.addEmbedded(); err != nil { + return nil, err + } + + for _, p := range paths { + data, err := os.ReadFile(p) + if err != nil { + return nil, fmt.Errorf("read chain file %s: %w", p, err) + } + if err := r.add(data, p); err != nil { + return nil, err + } + } + return r, nil +} + +// addEmbedded reads every chain file compiled into the binary. +func (r *Registry) addEmbedded() error { + entries, err := fs.ReadDir(embeddedChains, "chains") + if err != nil { + return fmt.Errorf("read embedded chains: %w", err) + } + for _, entry := range entries { + if entry.IsDir() || path.Ext(entry.Name()) != ".json" { + continue + } + name := path.Join("chains", entry.Name()) + data, err := embeddedChains.ReadFile(name) + if err != nil { + return fmt.Errorf("read embedded chain file %s: %w", name, err) + } + if err := r.add(data, embeddedSource); err != nil { + return err + } + } + return nil +} + +// add parses one chain file and records it under its identity, noting which +// source supplied it. A later source replaces an earlier one. +func (r *Registry) add(data []byte, source string) error { + var chain Chain + if err := decodeStrict(data, &chain); err != nil { + return fmt.Errorf("parse chain file %s: %w", source, err) + } + if err := chain.validate(); err != nil { + return fmt.Errorf("chain file %s: %w", source, err) + } + + key := chainKey{chainID: chain.ChainID, genesisHash: chain.GenesisHash} + r.chains[key] = chain + r.sources[key] = source + return nil +} + +// validate rejects a chain file missing a field the format requires. A file +// missing its identity would match nothing, and a contract missing its address +// would resolve to the zero address, which holds no code on any chain. +func (c Chain) validate() error { + if c.ChainID == 0 { + return errors.New("chainId is required") + } + if c.ChainName == "" { + return errors.New("chainName is required") + } + if c.GenesisHash == "" { + return errors.New("genesisHash is required") + } + for i, contract := range c.Contracts { + if contract.Name == "" { + return fmt.Errorf("contracts[%d]: name is required", i) + } + if contract.Address == (common.Address{}) { + return fmt.Errorf("contracts[%d] %s: address is required", + i, contract.Name) + } + if contract.CodeHash == (common.Hash{}) { + return fmt.Errorf("contracts[%d] %s: codeHash is required", + i, contract.Name) + } + } + return nil +} + +// Chain returns the entry matching both the chain id and the genesis hash. It +// reports false when no entry matches either. +func (r *Registry) Chain(chainID int64, genesisHash string) (Chain, bool) { + chain, ok := r.chains[chainKey{chainID: chainID, genesisHash: genesisHash}] + return chain, ok +} + +// Sources reports which file supplied each chain, keyed by chain name, for the +// run to log. A compiled-in chain reports "embedded". +func (r *Registry) Sources() map[string]string { + sources := make(map[string]string, len(r.sources)) + for key, source := range r.sources { + sources[r.chains[key].ChainName] = source + } + return sources +} + +// decodeStrict unmarshals JSON into v. It rejects a key that maps to no field, +// and data after the value, which json.Unmarshal also rejects. +// +// This mirrors config.decodeStrict deliberately. A chain file that silently +// ignores a misspelled key would resolve a contract nobody configured. +func decodeStrict(data []byte, v any) error { + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(v); err != nil { + return err + } + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + return errors.New("unexpected data after the top-level JSON value") + } + return nil +} diff --git a/registry/registry_test.go b/registry/registry_test.go new file mode 100644 index 0000000..0fcfa94 --- /dev/null +++ b/registry/registry_test.go @@ -0,0 +1,256 @@ +package registry_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/sei-protocol/sei-load/registry" +) + +// The fixture chains, by identity. A test naming one of these reads the file in +// testdata rather than building a Chain inline. +const ( + withContractPath = "testdata/with-contract.json" + noContractPath = "testdata/no-contract.json" + + withContractID = int64(713715) + withContractHash = "3f1a9c4e2b7d8f0a1c3e5b7d9f0a2c4e6b8d0f2a4c6e8b0d2f4a6c8e0b2d4f6a" + withContractName = "fixture-with-contract" + noContractID = int64(328) + noContractHash = "9f8e7d6c5b4a39281706f5e4d3c2b1a09f8e7d6c5b4a39281706f5e4d3c2b1a0" + fixtureContract = "storagerw" + fixtureAddressText = "0x1234567890123456789012345678901234567890" +) + +// TestLoadWithNoPathsReadsOnlyTheBinary asserts CDR-018: Load with no paths +// returns the compiled-in registry, and reaches for nothing else. +// +// chains/ ships with no chain files, so the registry is empty today. The +// assertion is that Load succeeds and finds nothing, not that it finds nothing +// forever. +func TestLoadWithNoPathsReadsOnlyTheBinary(t *testing.T) { + r, err := registry.Load() + if err != nil { + t.Fatalf("Load(): %v", err) + } + if got := len(r.Sources()); got != 0 { + t.Errorf("compiled-in registry holds %d chains, want 0. A chain file "+ + "in registry/chains/ needs its own test naming it.", got) + } +} + +// TestChainMatchesOnIdentityAlone asserts CDR-014 and CDR-015: a lookup matches +// on the chain id and the genesis hash together, and on nothing else. +func TestChainMatchesOnIdentityAlone(t *testing.T) { + r, err := registry.Load(withContractPath) + if err != nil { + t.Fatalf("Load(%s): %v", withContractPath, err) + } + + cases := []struct { + name string + chainID int64 + genesisHash string + want bool + }{ + {"both fields match", withContractID, withContractHash, true}, + {"right id, wrong hash", withContractID, noContractHash, false}, + {"wrong id, right hash", noContractID, withContractHash, false}, + {"neither matches", 1, "deadbeef", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, ok := r.Chain(tc.chainID, tc.genesisHash) + if ok != tc.want { + t.Errorf("Chain(%d, %q) matched %v, want %v", + tc.chainID, tc.genesisHash, ok, tc.want) + } + }) + } +} + +// TestChainNameAndURIAreNeverMatchedOn asserts the other half of CDR-015. Both +// fields exist for a human reading a failure, and a later reader must not turn +// either into a key. +func TestChainNameAndURIAreNeverMatchedOn(t *testing.T) { + renamed := filepath.Join(t.TempDir(), "renamed.json") + original, err := os.ReadFile(withContractPath) + if err != nil { + t.Fatalf("read fixture: %v", err) + } + edited := strings.ReplaceAll(string(original), withContractName, "renamed-chain") + edited = strings.ReplaceAll(edited, "s3://fixture-genesis-artifacts", "s3://elsewhere") + if err := os.WriteFile(renamed, []byte(edited), 0o600); err != nil { + t.Fatalf("write renamed fixture: %v", err) + } + + r, err := registry.Load(renamed) + if err != nil { + t.Fatalf("Load(renamed): %v", err) + } + chain, ok := r.Chain(withContractID, withContractHash) + if !ok { + t.Fatal("a chain with a different name and URI did not match its own " + + "identity, so one of those fields is being matched on") + } + if chain.ChainName != "renamed-chain" { + t.Errorf("ChainName = %q, want the edited name", chain.ChainName) + } +} + +// TestContractLookupReportsAMissRatherThanFailing asserts CDR-005 and the deploy +// case: a chain carrying no entry for a name reports false, which leads to a +// deployment rather than an error. +func TestContractLookupReportsAMissRatherThanFailing(t *testing.T) { + r, err := registry.Load(withContractPath, noContractPath) + if err != nil { + t.Fatalf("Load(both fixtures): %v", err) + } + + populated, ok := r.Chain(withContractID, withContractHash) + if !ok { + t.Fatal("the populated fixture chain did not match its own identity") + } + contract, ok := populated.Contract(fixtureContract) + if !ok { + t.Fatalf("Contract(%q) missed on a chain that carries it", fixtureContract) + } + if want := common.HexToAddress(fixtureAddressText); contract.Address != want { + t.Errorf("Address = %s, want %s", contract.Address, want) + } + if contract.CodeHash == (common.Hash{}) { + t.Error("CodeHash is the zero hash, so the fixture did not round-trip") + } + + if _, ok := populated.Contract("absent"); ok { + t.Error("Contract(\"absent\") matched, so the name is not being compared") + } + + empty, ok := r.Chain(noContractID, noContractHash) + if !ok { + t.Fatal("the empty fixture chain did not match its own identity") + } + if _, ok := empty.Contract(fixtureContract); ok { + t.Error("a chain carrying no contracts reported one") + } +} + +// TestSuppliedFileWinsOverTheBinary asserts CDR-019 and CDR-020: a supplied file +// replaces an entry for the same chain, and Sources reports which file won. +func TestSuppliedFileWinsOverTheBinary(t *testing.T) { + moved := filepath.Join(t.TempDir(), "moved.json") + original, err := os.ReadFile(withContractPath) + if err != nil { + t.Fatalf("read fixture: %v", err) + } + edited := strings.ReplaceAll(string(original), fixtureAddressText, + "0x9999999999999999999999999999999999999999") + if err := os.WriteFile(moved, []byte(edited), 0o600); err != nil { + t.Fatalf("write moved fixture: %v", err) + } + + r, err := registry.Load(withContractPath, moved) + if err != nil { + t.Fatalf("Load(fixture, moved): %v", err) + } + chain, ok := r.Chain(withContractID, withContractHash) + if !ok { + t.Fatal("the chain did not match after layering") + } + contract, ok := chain.Contract(fixtureContract) + if !ok { + t.Fatalf("Contract(%q) missed after layering", fixtureContract) + } + want := common.HexToAddress("0x9999999999999999999999999999999999999999") + if contract.Address != want { + t.Errorf("Address = %s, want %s. The last file supplied must win.", + contract.Address, want) + } + if got := r.Sources()[withContractName]; got != moved { + t.Errorf("Sources()[%q] = %q, want %q", withContractName, got, moved) + } +} + +// TestLoadRejectsAFileItCannotTrust asserts that a chain file which would +// resolve to the wrong thing fails at load rather than at send time. +// +// A misspelled key is the case worth naming: JSON that ignores it would leave a +// field at its zero value, and a contract at the zero address holds no code on +// any chain. +func TestLoadRejectsAFileItCannotTrust(t *testing.T) { + cases := []struct { + name string + body string + want string + }{ + { + name: "a misspelled key", + body: `{"chainId":1,"chainName":"x","genesisHash":"h","contract":[]}`, + want: "unknown field", + }, + { + name: "data after the value", + body: `{"chainId":1,"chainName":"x","genesisHash":"h","contracts":[]}{}`, + want: "unexpected data", + }, + { + name: "no chain id", + body: `{"chainName":"x","genesisHash":"h","contracts":[]}`, + want: "chainId is required", + }, + { + name: "no genesis hash", + body: `{"chainId":1,"chainName":"x","contracts":[]}`, + want: "genesisHash is required", + }, + { + name: "a contract with no address", + body: `{"chainId":1,"chainName":"x","genesisHash":"h","contracts":[{"name":"c","codeHash":"0x` + strings.Repeat("a", 64) + `"}]}`, + want: "address is required", + }, + { + name: "a contract with no name", + body: `{"chainId":1,"chainName":"x","genesisHash":"h","contracts":[{"address":"0x` + strings.Repeat("b", 40) + `","codeHash":"0x` + strings.Repeat("a", 64) + `"}]}`, + want: "name is required", + }, + { + name: "a contract with no code hash", + body: `{"chainId":1,"chainName":"x","genesisHash":"h","contracts":[{"name":"c","address":"0x` + strings.Repeat("b", 40) + `"}]}`, + want: "codeHash is required", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := filepath.Join(t.TempDir(), "chain.json") + if err := os.WriteFile(p, []byte(tc.body), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + _, err := registry.Load(p) + if err == nil { + t.Fatalf("Load accepted %s", tc.name) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not mention %q", err, tc.want) + } + }) + } +} + +// TestLoadReportsAMissingFile asserts a supplied path that does not exist fails +// with the path named, rather than silently falling back to the binary. +func TestLoadReportsAMissingFile(t *testing.T) { + absent := filepath.Join(t.TempDir(), "absent.json") + _, err := registry.Load(absent) + if err == nil { + t.Fatal("Load accepted a path that does not exist, so a typo in " + + "--chain-file would silently run against the compiled-in registry") + } + if !strings.Contains(err.Error(), absent) { + t.Errorf("error %q does not name the missing path", err) + } +} diff --git a/registry/testdata/README.md b/registry/testdata/README.md new file mode 100644 index 0000000..807face --- /dev/null +++ b/registry/testdata/README.md @@ -0,0 +1,11 @@ +# Registry test fixtures + +Two chains, so a test reads a fixture rather than building a `Chain` inline. + +- `with-contract.json` — one chain carrying one contract. The resolve and verify + paths read this. +- `no-contract.json` — one chain carrying none, and no `genesisS3URI`. This is + the deploy case, and it proves the optional field is optional. + +The addresses and hashes are not real. Nothing reads them from a chain; a test +supplies a fake `CodeReader` that returns whatever the case needs. diff --git a/registry/testdata/no-contract.json b/registry/testdata/no-contract.json new file mode 100644 index 0000000..dbd4e15 --- /dev/null +++ b/registry/testdata/no-contract.json @@ -0,0 +1,6 @@ +{ + "chainId": 328, + "chainName": "fixture-no-contract", + "genesisHash": "9f8e7d6c5b4a39281706f5e4d3c2b1a09f8e7d6c5b4a39281706f5e4d3c2b1a0", + "contracts": [] +} diff --git a/registry/testdata/with-contract.json b/registry/testdata/with-contract.json new file mode 100644 index 0000000..9af445e --- /dev/null +++ b/registry/testdata/with-contract.json @@ -0,0 +1,13 @@ +{ + "chainId": 713715, + "chainName": "fixture-with-contract", + "genesisHash": "3f1a9c4e2b7d8f0a1c3e5b7d9f0a2c4e6b8d0f2a4c6e8b0d2f4a6c8e0b2d4f6a", + "genesisS3URI": "s3://fixture-genesis-artifacts/fixture-with-contract/genesis.json", + "contracts": [ + { + "name": "storagerw", + "address": "0x1234567890123456789012345678901234567890", + "codeHash": "0xabcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + } + ] +} From 58a8f5f952b095d3f6194a3bfe89c077aae9d98f Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 10:24:50 -0700 Subject: [PATCH 2/9] feat(registry): resolve, verify, record and write chain entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the surface a run calls. The registry now answers "does this contract already exist on this chain?", and produces the entry to write when it did not. Verify hashes what eth_getCode serves and compares it to the recorded hash. A mismatch returns *MismatchError, which is a distinct type so a caller can tell a stale registry from a dial failure — the two need different responses from an operator, and collapsing them would send someone looking for the wrong problem. A test asserts that separation. An address holding no code hashes to the zero hash rather than to Keccak-256 of the empty string. One error type therefore covers both mismatches, and Error tells them apart. Resolve verifies before it returns an address, and reports a deploy for an ordinary miss rather than failing. A mismatch is an error and never a deploy signal: redeploying over a stale entry looks like a fix and is not, because it hides that the registry no longer describes the chain. WriteChain refuses to write into the compiled-in chains directory. The binary cannot know where its own source tree is, so the check matches the two paths a run can realistically be given from inside the repo, "registry/chains" and a bare "chains". It is a guardrail against that mistake rather than a sandbox, and the doc comment says so. Record reads the code back from the chain rather than hashing the compiled bytecode. Creation bytecode runs the constructor and returns the runtime code, and only the runtime code is what eth_getCode serves. It refuses an address holding nothing, so a deployment that did not take effect fails here instead of producing an entry no later run can verify. Both guards were verified failing before being trusted. The write refusal test runs from a temp working directory with parents created, so a broken guard fails by writing the file rather than by hitting a missing directory; neutering the check turns both refusal cases red. Two control cases assert a directory that merely contains the word is still allowed. Closes CDR-001, CDR-002, CDR-003, CDR-006, CDR-007, CDR-008, CDR-009, CDR-010, CDR-011. Verifiers: gofmt clean, go vet clean, staticcheck clean, 17 tests passing. Co-Authored-By: Claude Opus 5 (1M context) --- registry/resolve.go | 207 ++++++++++++++++++++++ registry/resolve_test.go | 372 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 579 insertions(+) create mode 100644 registry/resolve.go create mode 100644 registry/resolve_test.go diff --git a/registry/resolve.go b/registry/resolve.go new file mode 100644 index 0000000..1076452 --- /dev/null +++ b/registry/resolve.go @@ -0,0 +1,207 @@ +package registry + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "math/big" + "os" + "path" + "path/filepath" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" +) + +// CodeReader is the one chain call this package makes. *ethclient.Client +// satisfies it, and a test supplies a fake, so this package imports no Ethereum +// client of its own. +type CodeReader interface { + CodeAt(ctx context.Context, account common.Address, block *big.Int) ([]byte, error) +} + +// MismatchError reports that the code at a recorded address is not the code that +// was recorded. It is a distinct type so a caller can tell a stale registry from +// a dial failure. +// +// Got holds the zero hash when the address holds no code at all. One type +// therefore covers both cases, and Error tells them apart. +type MismatchError struct { + ChainName string + ChainID int64 + ContractName string + Address common.Address + Want common.Hash // recorded at deployment + Got common.Hash // observed now; the zero hash means no code +} + +func (e *MismatchError) Error() string { + if e.Got == (common.Hash{}) { + return fmt.Sprintf( + "%s (chain %d): contract %s is recorded at %s, and that address holds no code. "+ + "The registry entry is stale, most likely because the chain was re-genesised. "+ + "Recorded code hash %s.", + e.ChainName, e.ChainID, e.ContractName, e.Address, e.Want) + } + return fmt.Sprintf( + "%s (chain %d): contract %s at %s holds different code than recorded. "+ + "Recorded %s, found %s. The binding would encode against the wrong ABI.", + e.ChainName, e.ChainID, e.ContractName, e.Address, e.Want, e.Got) +} + +// Verify checks that the code at the recorded address hashes to the recorded +// hash. It returns a *MismatchError when it does not, and it never deploys. +// +// A run calls this before it sends any transaction. A call to an address holding +// no code succeeds at the EVM layer and does nothing, and a call to an address +// holding different code encodes against the wrong ABI. Neither shows up as a +// failed transaction, so neither is visible after the fact. +func Verify(ctx context.Context, code CodeReader, chain Chain, name string) error { + contract, ok := chain.Contract(name) + if !ok { + return fmt.Errorf("%s (chain %d): no contract named %s", + chain.ChainName, chain.ChainID, name) + } + + observed, err := codeHashAt(ctx, code, contract.Address) + if err != nil { + return fmt.Errorf("%s (chain %d): read code for %s at %s: %w", + chain.ChainName, chain.ChainID, name, contract.Address, err) + } + if observed == contract.CodeHash { + return nil + } + return &MismatchError{ + ChainName: chain.ChainName, + ChainID: chain.ChainID, + ContractName: name, + Address: contract.Address, + Want: contract.CodeHash, + Got: observed, + } +} + +// codeHashAt reads the runtime code at addr and hashes it the way the EVM +// defines an account's code hash. An address holding no code hashes to the zero +// hash rather than to Keccak-256 of the empty string, so a caller can tell +// "absent" from "present but different" by comparing against the zero value. +func codeHashAt(ctx context.Context, code CodeReader, addr common.Address) (common.Hash, error) { + runtime, err := code.CodeAt(ctx, addr, nil) + if err != nil { + return common.Hash{}, err + } + if len(runtime) == 0 { + return common.Hash{}, nil + } + return crypto.Keccak256Hash(runtime), nil +} + +// Resolve returns the address for a named contract, and reports whether the +// caller needs to deploy. It verifies before it returns an address. +// +// It returns (addr, false, nil) to bind, (zero, true, nil) to deploy, and an +// error when a recorded address failed verification. A profile that forces a +// deployment does not call it. +func Resolve(ctx context.Context, code CodeReader, r *Registry, + chainID int64, genesisHash, name string, +) (common.Address, bool, error) { + chain, ok := r.Chain(chainID, genesisHash) + if !ok { + return common.Address{}, true, nil + } + contract, ok := chain.Contract(name) + if !ok { + return common.Address{}, true, nil + } + if err := Verify(ctx, code, chain, name); err != nil { + return common.Address{}, false, err + } + return contract.Address, false, nil +} + +// Record reads the code at a freshly deployed address and returns the entry to +// write. It writes nothing itself. +// +// The code hash comes from the chain rather than from the compiled bytecode, +// because the two differ: creation bytecode runs the constructor and returns the +// runtime code, and only the runtime code is what eth_getCode serves. +func Record(ctx context.Context, code CodeReader, name string, + addr common.Address, +) (Contract, error) { + if name == "" { + return Contract{}, errors.New("record a contract: name is required") + } + if addr == (common.Address{}) { + return Contract{}, errors.New("record a contract: address is required") + } + + hash, err := codeHashAt(ctx, code, addr) + if err != nil { + return Contract{}, fmt.Errorf("record %s at %s: %w", name, addr, err) + } + if hash == (common.Hash{}) { + return Contract{}, fmt.Errorf( + "record %s at %s: that address holds no code, so the deployment did not take effect", + name, addr) + } + return Contract{Name: name, Address: addr, CodeHash: hash}, nil +} + +// embeddedDirName is the directory whose contents compile into the binary, and +// packageDirName is the package directory holding it. WriteChain refuses to +// write into "chains" or "registry/chains". +const ( + embeddedDirName = "chains" + packageDirName = "registry" +) + +// errWriteToEmbedded is what WriteChain returns for a path inside the +// compiled-in registry. +var errWriteToEmbedded = errors.New( + "refusing to write inside the compiled-in " + embeddedDirName + " directory") + +// checkWritePath refuses a path that names a file inside the compiled-in chains +// directory. That refusal is what makes the read-only rule enforceable rather +// than a convention: a run pointed at the committed registry still cannot edit +// it. +// +// The binary does not know where its own source tree is, so this matches the two +// paths a run can realistically be given from inside the repo: "registry/chains" +// and a bare "chains". It is a guardrail against that mistake, not a sandbox — +// an absolute path to the same directory under another name still writes. +func checkWritePath(p string) error { + dir := path.Dir(path.Clean(filepath.ToSlash(p))) + if dir == embeddedDirName || path.Base(dir) == embeddedDirName && + path.Base(path.Dir(dir)) == packageDirName { + return fmt.Errorf("%s: %w", p, errWriteToEmbedded) + } + return nil +} + +// WriteChain writes a chain file for an operator to review and commit. It +// refuses a path inside the compiled-in chains directory, and it writes nothing +// when it refuses. +// +// The file it writes is the same format Load reads, so a run can be pointed at +// its own output. Load validates what it reads, so a file this function wrote is +// a file Load accepts. +func WriteChain(p string, chain Chain) error { + if err := checkWritePath(p); err != nil { + return err + } + if err := chain.validate(); err != nil { + return fmt.Errorf("write chain file %s: %w", p, err) + } + + data, err := json.MarshalIndent(chain, "", " ") + if err != nil { + return fmt.Errorf("write chain file %s: %w", p, err) + } + data = append(data, '\n') + + if err := os.WriteFile(p, data, 0o600); err != nil { + return fmt.Errorf("write chain file %s: %w", p, err) + } + return nil +} diff --git a/registry/resolve_test.go b/registry/resolve_test.go new file mode 100644 index 0000000..c1ea665 --- /dev/null +++ b/registry/resolve_test.go @@ -0,0 +1,372 @@ +package registry_test + +import ( + "context" + "errors" + "math/big" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/sei-protocol/sei-load/registry" +) + +// fakeCode is a CodeReader a test drives. It serves code per address, and counts +// its calls so a test can assert the startup request count. +type fakeCode struct { + code map[common.Address][]byte + err error + + calls int + seen []common.Address +} + +func (f *fakeCode) CodeAt(_ context.Context, addr common.Address, _ *big.Int) ([]byte, error) { + f.calls++ + f.seen = append(f.seen, addr) + if f.err != nil { + return nil, f.err + } + return f.code[addr], nil +} + +// serving returns a reader that serves runtime for addr and nothing elsewhere. +func serving(addr common.Address, runtime []byte) *fakeCode { + return &fakeCode{code: map[common.Address][]byte{addr: runtime}} +} + +// chainRecording returns a chain whose one contract records the hash of runtime, +// so Verify against a reader serving runtime succeeds. +func chainRecording(runtime []byte, addr common.Address) registry.Chain { + return registry.Chain{ + ChainID: withContractID, + ChainName: withContractName, + GenesisHash: withContractHash, + Contracts: []registry.Contract{{ + Name: fixtureContract, + Address: addr, + CodeHash: crypto.Keccak256Hash(runtime), + }}, + } +} + +// TestVerifyAcceptsTheCodeItRecorded asserts CDR-006 and CDR-007: Verify hashes +// the code the chain serves and compares it to the recorded hash. +func TestVerifyAcceptsTheCodeItRecorded(t *testing.T) { + runtime := []byte{0x60, 0x80, 0x60, 0x40, 0x52} + addr := common.HexToAddress(fixtureAddressText) + chain := chainRecording(runtime, addr) + + if err := registry.Verify(context.Background(), serving(addr, runtime), chain, fixtureContract); err != nil { + t.Fatalf("Verify rejected the code it recorded: %v", err) + } +} + +// TestVerifyReportsAbsentCodeAndWrongCodeApart asserts CDR-008: one error type +// covers both mismatches, and the message tells them apart. +func TestVerifyReportsAbsentCodeAndWrongCodeApart(t *testing.T) { + runtime := []byte{0x60, 0x80, 0x60, 0x40, 0x52} + addr := common.HexToAddress(fixtureAddressText) + chain := chainRecording(runtime, addr) + + cases := []struct { + name string + reader registry.CodeReader + wantGotZero bool + wantPhrase string + }{ + { + name: "the address holds no code", + reader: &fakeCode{code: map[common.Address][]byte{}}, + wantGotZero: true, + wantPhrase: "holds no code", + }, + { + name: "the address holds different code", + reader: serving(addr, []byte{0xfe, 0xfe}), + wantGotZero: false, + wantPhrase: "different code than recorded", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := registry.Verify(context.Background(), tc.reader, chain, fixtureContract) + if err == nil { + t.Fatal("Verify accepted a mismatch") + } + + var mismatch *registry.MismatchError + if !errors.As(err, &mismatch) { + t.Fatalf("error is %T, want *registry.MismatchError. A caller "+ + "cannot tell a stale registry from a dial failure.", err) + } + if got := mismatch.Got == (common.Hash{}); got != tc.wantGotZero { + t.Errorf("Got is zero = %v, want %v", got, tc.wantGotZero) + } + if !strings.Contains(err.Error(), tc.wantPhrase) { + t.Errorf("message %q does not contain %q", err, tc.wantPhrase) + } + + // CDR-008: the message names all four facts a reader needs. + for _, want := range []string{ + chain.ChainName, + fixtureContract, + addr.String(), + mismatch.Want.String(), + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("message %q does not name %q", err, want) + } + } + }) + } +} + +// TestVerifyDistinguishesAReadFailureFromAMismatch asserts CDR-008's purpose. A +// dial failure must not read as a stale registry, because the two need different +// responses from an operator. +func TestVerifyDistinguishesAReadFailureFromAMismatch(t *testing.T) { + runtime := []byte{0x60, 0x80} + addr := common.HexToAddress(fixtureAddressText) + chain := chainRecording(runtime, addr) + + wantErr := errors.New("dial tcp: connection refused") + err := registry.Verify(context.Background(), &fakeCode{err: wantErr}, chain, fixtureContract) + if err == nil { + t.Fatal("Verify succeeded against a reader that cannot read") + } + + var mismatch *registry.MismatchError + if errors.As(err, &mismatch) { + t.Error("a read failure surfaced as *MismatchError, so an operator " + + "would go looking for a stale entry instead of a broken endpoint") + } + if !errors.Is(err, wantErr) { + t.Errorf("error %q does not wrap the reader's own error", err) + } +} + +// TestResolveReportsDeployRatherThanFailing asserts CDR-003: a chain the registry +// does not cover, and a contract it does not name, both lead to a deployment. +func TestResolveReportsDeployRatherThanFailing(t *testing.T) { + r, err := registry.Load(withContractPath) + if err != nil { + t.Fatalf("Load: %v", err) + } + code := &fakeCode{code: map[common.Address][]byte{}} + + cases := []struct { + name string + chainID int64 + genesisHash string + contract string + }{ + {"the registry does not cover the chain", 999, "unknown", fixtureContract}, + {"the chain does not name the contract", withContractID, withContractHash, "absent"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + addr, deploy, err := registry.Resolve(context.Background(), code, r, + tc.chainID, tc.genesisHash, tc.contract) + if err != nil { + t.Fatalf("Resolve returned an error for an ordinary miss: %v", err) + } + if !deploy { + t.Error("Resolve did not report that the caller needs to deploy") + } + if addr != (common.Address{}) { + t.Errorf("Resolve returned address %s alongside a deploy signal", addr) + } + }) + } + + if code.calls != 0 { + t.Errorf("Resolve issued %d code reads for a chain it has no entry for, want 0", + code.calls) + } +} + +// TestResolveVerifiesBeforeItReturnsAnAddress asserts CDR-002 and CDR-006: an +// address the caller would bind is verified first, and a mismatch is an error +// rather than a deploy signal. +func TestResolveVerifiesBeforeItReturnsAnAddress(t *testing.T) { + runtime := []byte{0x60, 0x80, 0x60, 0x40} + addr := common.HexToAddress(fixtureAddressText) + + // Write a fixture recording the hash of runtime, so the happy path matches. + chain := chainRecording(runtime, addr) + p := filepath.Join(t.TempDir(), "chain.json") + if err := registry.WriteChain(p, chain); err != nil { + t.Fatalf("WriteChain: %v", err) + } + r, err := registry.Load(p) + if err != nil { + t.Fatalf("Load: %v", err) + } + + t.Run("the code matches", func(t *testing.T) { + got, deploy, err := registry.Resolve(context.Background(), serving(addr, runtime), r, + withContractID, withContractHash, fixtureContract) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if deploy { + t.Error("Resolve reported a deploy for an entry it could verify") + } + if got != addr { + t.Errorf("Resolve returned %s, want %s", got, addr) + } + }) + + t.Run("the address holds nothing", func(t *testing.T) { + _, deploy, err := registry.Resolve(context.Background(), + &fakeCode{code: map[common.Address][]byte{}}, r, + withContractID, withContractHash, fixtureContract) + if err == nil { + t.Fatal("Resolve accepted an address holding no code") + } + if deploy { + t.Error("Resolve reported a deploy after a mismatch. CDR-009 forbids " + + "redeploying over a stale entry: it hides the staleness.") + } + }) +} + +// TestRecordReadsTheCodeTheChainServes asserts CDR-010: the recorded hash comes +// from the chain, not from the compiled bytecode. +func TestRecordReadsTheCodeTheChainServes(t *testing.T) { + runtime := []byte{0x60, 0x80, 0x60, 0x40, 0x52, 0x34} + addr := common.HexToAddress(fixtureAddressText) + + contract, err := registry.Record(context.Background(), serving(addr, runtime), + fixtureContract, addr) + if err != nil { + t.Fatalf("Record: %v", err) + } + if contract.Name != fixtureContract { + t.Errorf("Name = %q, want %q", contract.Name, fixtureContract) + } + if contract.Address != addr { + t.Errorf("Address = %s, want %s", contract.Address, addr) + } + if want := crypto.Keccak256Hash(runtime); contract.CodeHash != want { + t.Errorf("CodeHash = %s, want %s (Keccak-256 of the runtime code)", + contract.CodeHash, want) + } +} + +// TestRecordRejectsAnAddressHoldingNothing asserts that a deployment which did +// not take effect fails here rather than producing an entry no run can verify. +func TestRecordRejectsAnAddressHoldingNothing(t *testing.T) { + addr := common.HexToAddress(fixtureAddressText) + _, err := registry.Record(context.Background(), + &fakeCode{code: map[common.Address][]byte{}}, fixtureContract, addr) + if err == nil { + t.Fatal("Record accepted an address holding no code, so it would write " + + "an entry that fails every later run") + } + if !strings.Contains(err.Error(), "holds no code") { + t.Errorf("error %q does not say why", err) + } +} + +// TestWriteChainRefusesTheCompiledInDirectory asserts CDR-011, and that the +// refusal writes nothing. Asserting only the success path would leave the rule a +// convention. +// +// Each case runs from a temp working directory with the target's parents already +// created, so a broken guard fails this test by writing the file rather than by +// hitting a missing directory. The two control cases prove that. +func TestWriteChainRefusesTheCompiledInDirectory(t *testing.T) { + runtime := []byte{0x60, 0x80} + chain := chainRecording(runtime, common.HexToAddress(fixtureAddressText)) + + // The paths a run can realistically be given from inside the repo: from the + // repo root, and from the registry package directory. + refused := []string{ + filepath.Join("registry", "chains", "arctic-1.json"), + filepath.Join("chains", "arctic-1.json"), + } + // Allowed: a directory that is not the compiled-in one, however it is named. + // The guard protects a location, not a word. + allowed := []string{ + filepath.Join("my-chains", "arctic-1.json"), + filepath.Join("out", "chains-backup", "arctic-1.json"), + } + + run := func(t *testing.T, rel string) error { + t.Helper() + t.Chdir(t.TempDir()) + if err := os.MkdirAll(filepath.Dir(rel), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + return registry.WriteChain(rel, chain) + } + + for _, rel := range refused { + t.Run("refused "+rel, func(t *testing.T) { + err := run(t, rel) + if err == nil { + t.Fatalf("WriteChain accepted %s, so a run could edit the "+ + "committed registry", rel) + } + if !strings.Contains(err.Error(), "refusing to write") { + t.Errorf("error %q does not say it refused, so it may have "+ + "failed for an unrelated reason", err) + } + if _, statErr := os.Stat(rel); statErr == nil { + t.Error("WriteChain refused and wrote the file anyway") + } + }) + } + + for _, rel := range allowed { + t.Run("allowed "+rel, func(t *testing.T) { + if err := run(t, rel); err != nil { + t.Fatalf("WriteChain refused a legitimate path: %v", err) + } + if _, err := os.Stat(rel); err != nil { + t.Errorf("WriteChain reported success and wrote nothing: %v", err) + } + }) + } +} + +// TestWriteChainRoundTripsThroughLoad asserts a written file is a file Load +// accepts. A run pointed at its own output must work, because that is the +// operator workflow: run, review, commit. +func TestWriteChainRoundTripsThroughLoad(t *testing.T) { + runtime := []byte{0x60, 0x80, 0x60, 0x40} + addr := common.HexToAddress(fixtureAddressText) + chain := chainRecording(runtime, addr) + chain.GenesisS3URI = "s3://bucket/genesis.json" + + p := filepath.Join(t.TempDir(), "written.json") + if err := registry.WriteChain(p, chain); err != nil { + t.Fatalf("WriteChain: %v", err) + } + + r, err := registry.Load(p) + if err != nil { + t.Fatalf("Load could not read what WriteChain wrote: %v", err) + } + got, ok := r.Chain(withContractID, withContractHash) + if !ok { + t.Fatal("the written chain did not match its own identity after reloading") + } + if got.GenesisS3URI != chain.GenesisS3URI { + t.Errorf("GenesisS3URI = %q, want %q", got.GenesisS3URI, chain.GenesisS3URI) + } + contract, ok := got.Contract(fixtureContract) + if !ok { + t.Fatal("the written contract is missing after reloading") + } + if contract.CodeHash != crypto.Keccak256Hash(runtime) { + t.Errorf("CodeHash did not survive the round trip") + } +} From a8b2534fffd2583f099e0ff643ed9e44d5da00af Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 10:35:11 -0700 Subject: [PATCH 3/9] feat(generator): resolve contracts from the registry before deploying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run now asks the registry for each contract its profile drives, deploys only what the registry does not name, and records what it deployed. A stale entry stops the run at startup instead of corrupting it. Removing AttachScenario found more than the spec described. Each of the six contract scenarios overrode Attach to dial and bind — and that override called the base, which dialed and bound again. Every contract scenario was dialing twice and binding twice on attach. Both paths are gone. What replaces them: Ready(config) marks a scenario able to generate, and Binder() returns a closure that binds one contract and stores the instance. The closure is built where the contract type is known, so the preparation step drives it without knowing that type, and the step owns the backend and the address. No scenario dials, and none panics. ScenarioBase no longer has an address field, and GetAddress is gone. Both were dead — nothing outside the type read either — and deleting them makes CDR-021 structurally true rather than a comment: a scenario cannot hold what does not exist. staticcheck flagged the leftover write, which is what surfaced it. Address precedence is fixed and explicit: a configured contractAddress, then forceDeploy, then a registry entry, then a deployment because nothing named one. Every assertion was verified failing before being trusted. The no-deployment test has a control that points the fixture at a different address and asserts the run refuses to start. The CI gate on committed chain files was checked against a deliberately malformed file. Closes CDR-004, CDR-012, CDR-013, CDR-016, CDR-019, CDR-020, CDR-021, CDR-022, CDR-023, and the T014/T019/T022/T023/T026/T027/T028 assertions. Verifiers: gofmt clean, go vet clean, staticcheck clean, golangci-lint 0 issues, all 14 packages passing. check-bindings is CI-only on Darwin and this change touches no .sol file. Co-Authored-By: Claude Opus 5 (1M context) --- config/config.go | 23 ++ generator/deploy_test.go | 2 +- generator/generator.go | 58 ++--- generator/mockchain_test.go | 30 ++- generator/prepare.go | 243 +++++++++++++++++++ generator/registry_test.go | 329 ++++++++++++++++++++++++++ generator/scenarios/Disperse.go | 20 -- generator/scenarios/ERC20.go | 20 -- generator/scenarios/ERC20Conflict.go | 20 -- generator/scenarios/ERC20Noop.go | 20 -- generator/scenarios/ERC721.go | 20 -- generator/scenarios/StorageRW.go | 28 --- generator/scenarios/StorageRW_test.go | 8 +- generator/scenarios/base.go | 75 +++--- main.go | 11 + registry/chains_test.go | 49 ++++ 16 files changed, 746 insertions(+), 210 deletions(-) create mode 100644 generator/prepare.go create mode 100644 generator/registry_test.go create mode 100644 registry/chains_test.go diff --git a/config/config.go b/config/config.go index 042292a..a0735c3 100644 --- a/config/config.go +++ b/config/config.go @@ -13,6 +13,19 @@ import ( // LoadConfig stores the configuration for load-related settings. type LoadConfig struct { ChainID int64 `json:"chainId,omitempty"` + // GenesisHash is the other half of the chain's identity, and the registry + // matches on it alongside ChainID. An EVM chain id alone does not identify a + // chain instance: a devnet keeps its id across a re-genesis, so an entry + // recorded before the re-genesis names an address that no longer holds its + // contract. Bare hex, matching SeiNetwork.Status.GenesisHash. + GenesisHash string `json:"genesisHash,omitempty"` + // ChainFiles are contract registry files the deployment supplies, layered + // over the registry compiled into the binary in the order given. The + // --chain-file flag appends to this. + ChainFiles []string `json:"chainFiles,omitempty"` + // ChainRecordPath is where a run writes what it deployed, as a chain file an + // operator reviews and commits. Empty writes nothing. + ChainRecordPath string `json:"chainRecordPath,omitempty"` // SeiChainID is the textual chain ID used for tagging metric collection. SeiChainID string `json:"seiChainID,omitempty"` Endpoints []string `json:"endpoints"` @@ -171,6 +184,16 @@ type Scenario struct { // by operation name. Absent (the default) selects the scenario's first // declared operation; see operation.go. Operations OperationMix `json:"operations,omitempty"` + // ContractAddress names a contract deployed outside this repo. Set, the run + // binds it and consults no registry and deploys nothing. It is the escape + // hatch for a contract the registry does not and should not describe. + ContractAddress string `json:"contractAddress,omitempty"` + // ForceDeploy deploys a fresh contract even where the registry holds an + // entry for this chain. It exists for measuring deployment itself, and for + // a run that must not touch state another run has already written. + // + // It is an opt-out, not a mode: everything after resolution is unchanged. + ForceDeploy bool `json:"forceDeploy,omitempty"` } const ( diff --git a/generator/deploy_test.go b/generator/deploy_test.go index af66c4b..3f05efd 100644 --- a/generator/deploy_test.go +++ b/generator/deploy_test.go @@ -87,7 +87,7 @@ func TestDeployFailureIsAnError(t *testing.T) { chain := newMockChain(t, mockChainConfig{revertDeployments: true}) _, err := generator.NewGenerator(t.Context(), newTestRng(1), contractConfig(chain), deployer) - require.ErrorContains(t, err, "failed to deploy scenarios") + require.ErrorContains(t, err, "failed to prepare scenarios") require.ErrorContains(t, err, scenarios.StorageRW) require.ErrorContains(t, err, "deployment transaction failed with status 0") } diff --git a/generator/generator.go b/generator/generator.go index 5e271ed..ef6c927 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -25,6 +25,9 @@ type scenarioInstance struct { Weight int Scenario scenarios.TxGenerator Accounts *types.AccountPool + // Config is the profile entry that produced this instance. The preparation + // step reads it to decide which address the scenario binds. + Config config.Scenario } // generatorBuilder manages scenario creation and deployment from config @@ -83,6 +86,7 @@ func (g *generatorBuilder) createScenarios() error { Weight: scenarioCfg.Weight, Scenario: scenario, Accounts: accountPool, + Config: scenarioCfg, } g.instances = append(g.instances, instance) @@ -91,44 +95,6 @@ func (g *generatorBuilder) createScenarios() error { return nil } -// mockDeployAll deploys all scenario instances that require deployment (for unit tests). -func (g *generatorBuilder) mockDeployAll() error { - for _, instance := range g.instances { - addr := types.NewAccount(false).Address - if err := instance.Scenario.Attach(g.config, addr); err != nil { - return err - } - } - return nil -} - -// deployAll deploys all scenario instances that require deployment, from the -// deployer the run was handed. Sequential by design (see package doc): each -// deployment reads its nonce from the chain and is mined before the next is -// sent, so one deployer key stays in one ordered nonce stream. -func (g *generatorBuilder) deployAll(ctx context.Context, deployer types.Account) error { - if g.config.MockDeploy { - return g.mockDeployAll() - } - if deployer.PrivKey == nil { - return errors.New("deployer has no private key (a live deployment must be signed)") - } - - log.Printf("Deploying %d scenarios from %s", len(g.instances), deployer.Address.Hex()) - for _, instance := range g.instances { - log.Printf("Deploying scenario %s", instance.Name) - address, err := instance.Scenario.Deploy(ctx, g.config, deployer) - if err != nil { - return fmt.Errorf("deploy %s: %w", instance.Name, err) - } - if address != (common.Address{}) { - log.Printf("🚀 Deployed %s at address: %s\n", instance.Name, address.Hex()) - } - } - - return nil -} - type Generator struct{ scenarios []*scenarioInstance } func (g *Generator) Accounts() []types.Account { @@ -151,9 +117,9 @@ type TxSender interface { func (g *Generator) Prewarm(ctx context.Context, rng *mrand.Rand, cfg *config.LoadConfig, txSender TxSender) error { // Create EVMTransfer scenario for prewarming evmScenario := scenarios.NewEVMTransferScenario(config.Scenario{}) - // EVMTransfer needs no contract, so attaching is all that marks it ready. - if err := evmScenario.Attach(cfg, common.Address{}); err != nil { - return fmt.Errorf("evmScenario.Attach(): %w", err) + // EVMTransfer drives no contract, so marking it ready is all it needs. + if err := evmScenario.Ready(cfg); err != nil { + return fmt.Errorf("evmScenario.Ready(): %w", err) } for _, account := range g.Accounts() { // Create self-transfer transaction @@ -269,9 +235,13 @@ func NewGenerator(ctx context.Context, rng *mrand.Rand, cfg *config.LoadConfig, return nil, fmt.Errorf("failed to create scenarios: %w", err) } - // Step 2: Deploy all scenarios - if err := b.deployAll(ctx, deployer); err != nil { - return nil, fmt.Errorf("failed to deploy scenarios: %w", err) + // Step 2: give every scenario the contract it drives + prepared, err := b.prepareAll(ctx, deployer) + if err != nil { + return nil, fmt.Errorf("failed to prepare scenarios: %w", err) + } + if err := b.recordDeployments(ctx, prepared); err != nil { + return nil, fmt.Errorf("failed to record what was deployed: %w", err) } // Step 3: Create weighted scenarioGenerator diff --git a/generator/mockchain_test.go b/generator/mockchain_test.go index 8dc945d..8e32a42 100644 --- a/generator/mockchain_test.go +++ b/generator/mockchain_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "math/big" "net/http/httptest" + "slices" "testing" "github.com/ethereum/go-ethereum/common" @@ -24,8 +25,15 @@ type mockChainConfig struct { baseNonce map[common.Address]uint64 // revertDeployments mines every contract creation with a failed status. revertDeployments bool + // code serves per-address runtime code, so a test can make a recorded + // address hold nothing or hold the wrong contract. Nil serves defaultCode + // everywhere, which is what a deployment test wants. + code map[common.Address][]byte } +// defaultCode is what GetCode serves when a test sets no per-address code. +var defaultCode = []byte{0x60, 0x00} + // mockChain serves the smallest eth JSON-RPC surface a deployment needs. It // mines every transaction on arrival, reports a sender's pending nonce as its // base plus the transactions that sender has sent, and keeps what it received. @@ -39,6 +47,9 @@ type mockChain struct { type mockChainState struct { mined []minedTx byHash map[common.Hash]minedTx + // codeReads records every address GetCode was asked for, in order, so a test + // can assert the startup read count and which contracts it covered. + codeReads []common.Address } // minedTx is one transaction the chain accepted. contract is the created @@ -121,8 +132,23 @@ func (m *mockChain) GetBalance(_ context.Context, _ common.Address, _ rpc.BlockN return (*hexutil.Big)(new(big.Int)), nil } -func (m *mockChain) GetCode(_ context.Context, _ common.Address, _ rpc.BlockNumberOrHash) (hexutil.Bytes, error) { - return hexutil.Bytes{0x60, 0x00}, nil +func (m *mockChain) GetCode(_ context.Context, addr common.Address, _ rpc.BlockNumberOrHash) (hexutil.Bytes, error) { + for state := range m.state.Lock() { + state.codeReads = append(state.codeReads, addr) + } + if m.cfg.code == nil { + return defaultCode, nil + } + return m.cfg.code[addr], nil +} + +// codeReads returns every address GetCode was asked for, in order. +func (m *mockChain) codeReads() []common.Address { + var reads []common.Address + for state := range m.state.Lock() { + reads = slices.Clone(state.codeReads) + } + return reads } func (m *mockChain) EstimateGas(_ context.Context, _ json.RawMessage, _ *rpc.BlockNumberOrHash) (hexutil.Uint64, error) { diff --git a/generator/prepare.go b/generator/prepare.go new file mode 100644 index 0000000..bf5853a --- /dev/null +++ b/generator/prepare.go @@ -0,0 +1,243 @@ +package generator + +import ( + "context" + "errors" + "fmt" + "log" + + "github.com/ethereum/go-ethereum/accounts/abi/bind" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethclient" + + "github.com/sei-protocol/sei-load/registry" + "github.com/sei-protocol/sei-load/types" +) + +// prepared reports what one scenario instance ended up bound to, so the caller +// can record a deployment and log which source supplied an address. +type prepared struct { + instance *scenarioInstance + address common.Address + // deployed is true when this run created the contract, which is what makes + // it worth recording. A resolved address is already recorded. + deployed bool +} + +// prepareAll gives every scenario instance the contract it drives, and reports +// what it deployed. +// +// One client serves the whole step. It reads code, and it binds. It is not the +// client that sends load: CreateTransactionOpts sets auth.NoSend, so a bound +// contract hands the transaction back rather than sending it, and the sender's +// own per-endpoint clients send. +// +// The step resolves only the contracts the profile drives. A contract in the +// registry that no scenario needs is never read and never verified. +func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) ([]prepared, error) { + if g.config.MockDeploy { + return nil, g.mockPrepareAll() + } + + reg, err := registry.Load(g.config.ChainFiles...) + if err != nil { + return nil, fmt.Errorf("load the contract registry: %w", err) + } + logRegistrySources(reg) + + client, err := ethclient.Dial(g.config.Endpoints[0]) + if err != nil { + return nil, fmt.Errorf("dial %s: %w", g.config.Endpoints[0], err) + } + defer client.Close() + + results := make([]prepared, 0, len(g.instances)) + for _, instance := range g.instances { + result, err := g.prepareOne(ctx, reg, client, instance, deployer) + if err != nil { + return nil, fmt.Errorf("prepare %s: %w", instance.Name, err) + } + results = append(results, result) + } + return results, nil +} + +// prepareOne resolves, deploys if needed, and binds one scenario instance. +// +// A scenario that drives no contract is marked ready and nothing else. That is +// what keeps a registry lookup from happening for an ETH transfer. +func (g *generatorBuilder) prepareOne(ctx context.Context, reg *registry.Registry, + client *ethclient.Client, instance *scenarioInstance, deployer types.Account, +) (prepared, error) { + binder := instance.Scenario.Binder() + if binder == nil { + return prepared{instance: instance}, instance.Scenario.Ready(g.config) + } + + address, deployed, err := g.addressFor(ctx, reg, client, instance, deployer) + if err != nil { + return prepared{}, err + } + if err := instance.Scenario.Ready(g.config); err != nil { + return prepared{}, err + } + if err := binder(client, address); err != nil { + return prepared{}, err + } + return prepared{instance: instance, address: address, deployed: deployed}, nil +} + +// addressFor decides which address a contract scenario binds, in a fixed order +// of precedence: an explicit address from the profile, a forced deployment, a +// registry entry, then a deployment because nothing named one. +func (g *generatorBuilder) addressFor(ctx context.Context, reg *registry.Registry, + client *ethclient.Client, instance *scenarioInstance, deployer types.Account, +) (common.Address, bool, error) { + cfg := instance.Config + + if cfg.ContractAddress != "" { + if !common.IsHexAddress(cfg.ContractAddress) { + return common.Address{}, false, fmt.Errorf( + "contractAddress %q is not an address", cfg.ContractAddress) + } + address := common.HexToAddress(cfg.ContractAddress) + log.Printf("📌 %s: binding the configured address %s", instance.Name, address) + return address, false, nil + } + + if cfg.ForceDeploy { + log.Printf("🔁 %s: forceDeploy is set, deploying rather than resolving", instance.Name) + address, err := g.deployOne(ctx, instance, deployer) + return address, true, err + } + + address, mustDeploy, err := registry.Resolve(ctx, client, reg, + g.config.ChainID, g.config.GenesisHash, contractNameFor(instance)) + if err != nil { + return common.Address{}, false, err + } + if !mustDeploy { + log.Printf("📖 %s: bound the recorded address %s, no deployment sent", + instance.Name, address) + return address, false, nil + } + + address, err = g.deployOne(ctx, instance, deployer) + return address, true, err +} + +// deployOne deploys the contract one instance drives. It keeps the sequential +// deployment the package doc describes: one deployer key stays in one ordered +// nonce stream. +func (g *generatorBuilder) deployOne(ctx context.Context, instance *scenarioInstance, + deployer types.Account, +) (common.Address, error) { + if deployer.PrivKey == nil { + return common.Address{}, errors.New( + "deployer has no private key (a live deployment must be signed)") + } + log.Printf("Deploying scenario %s", instance.Name) + address, err := instance.Scenario.Deploy(ctx, g.config, deployer) + if err != nil { + return common.Address{}, err + } + if address != (common.Address{}) { + log.Printf("🚀 Deployed %s at address: %s", instance.Name, address) + } + return address, nil +} + +// contractNameFor is the key a scenario's contract is recorded under. The +// scenario's configured name is that key, so a profile and a chain file agree +// without a second vocabulary to keep in step. +func contractNameFor(instance *scenarioInstance) string { + return instance.Config.Name +} + +// logRegistrySources says which file supplied each chain the registry holds. A +// run against the wrong chain file is otherwise silent. +func logRegistrySources(reg *registry.Registry) { + sources := reg.Sources() + if len(sources) == 0 { + log.Printf("📖 contract registry: no chains, so every contract deploys") + return + } + for name, source := range sources { + log.Printf("📖 contract registry: %s from %s", name, source) + } +} + +// mockPrepareAll marks every scenario ready and binds each contract against a +// nil backend, which is the unit-test path. A nil backend builds transactions +// and never sends one. +func (g *generatorBuilder) mockPrepareAll() error { + for _, instance := range g.instances { + if err := instance.Scenario.Ready(g.config); err != nil { + return err + } + if binder := instance.Scenario.Binder(); binder != nil { + addr := types.NewAccount(false).Address + if err := binder(nilBackend(), addr); err != nil { + return err + } + } + } + return nil +} + +// nilBackend is the backend the mock path binds against. abigen stores it and +// touches it only when a call needs the chain, which the mock path never does. +func nilBackend() bind.ContractBackend { return nil } + +// recordDeployments writes a chain file describing what this run deployed, for +// an operator to review and commit. +// +// It writes nothing when the run deployed nothing, and nothing when the profile +// names no path. The file is not committed automatically, and an ephemeral +// chain's file should not be committed at all: that chain disappears after the +// run, so an entry naming it could never verify again. +func (g *generatorBuilder) recordDeployments(ctx context.Context, results []prepared) error { + if g.config.MockDeploy || g.config.ChainRecordPath == "" { + return nil + } + + deployed := make([]prepared, 0, len(results)) + for _, result := range results { + if result.deployed { + deployed = append(deployed, result) + } + } + if len(deployed) == 0 { + return nil + } + + client, err := ethclient.Dial(g.config.Endpoints[0]) + if err != nil { + return fmt.Errorf("dial %s: %w", g.config.Endpoints[0], err) + } + defer client.Close() + + chain := registry.Chain{ + ChainID: g.config.ChainID, + ChainName: g.config.SeiChainID, + GenesisHash: g.config.GenesisHash, + } + if chain.ChainName == "" { + chain.ChainName = fmt.Sprintf("chain-%d", g.config.ChainID) + } + for _, result := range deployed { + contract, err := registry.Record(ctx, client, + contractNameFor(result.instance), result.address) + if err != nil { + return err + } + chain.Contracts = append(chain.Contracts, contract) + } + + if err := registry.WriteChain(g.config.ChainRecordPath, chain); err != nil { + return err + } + log.Printf("📝 wrote %d deployed contract(s) to %s — review it before committing", + len(chain.Contracts), g.config.ChainRecordPath) + return nil +} diff --git a/generator/registry_test.go b/generator/registry_test.go new file mode 100644 index 0000000..959ced7 --- /dev/null +++ b/generator/registry_test.go @@ -0,0 +1,329 @@ +package generator_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/require" + + "github.com/sei-protocol/sei-load/config" + "github.com/sei-protocol/sei-load/generator" + "github.com/sei-protocol/sei-load/generator/scenarios" + "github.com/sei-protocol/sei-load/registry" + "github.com/sei-protocol/sei-load/types" +) + +const ( + testChainID = int64(7777) + testGenesisHash = "aa11bb22cc33dd44ee55ff6600778899aa11bb22cc33dd44ee55ff6600778899" + testChainName = "fixture-chain" +) + +// recordedCode is the runtime a recorded contract is expected to hold. The mock +// chain serves it, and the chain file records its hash, so verification passes. +var recordedCode = []byte{0x60, 0x80, 0x60, 0x40, 0x52} + +// chainFileFor writes a chain file naming one contract at addr, and returns its +// path. The recorded hash is the hash of runtime, so a chain serving runtime at +// addr verifies. +func chainFileFor(t *testing.T, contract string, addr common.Address, runtime []byte) string { + t.Helper() + path := filepath.Join(t.TempDir(), "chain.json") + require.NoError(t, registry.WriteChain(path, registry.Chain{ + ChainID: testChainID, + ChainName: testChainName, + GenesisHash: testGenesisHash, + Contracts: []registry.Contract{{ + Name: contract, + Address: addr, + CodeHash: crypto.Keccak256Hash(runtime), + }}, + })) + return path +} + +// oneContractConfig is a single-scenario profile, so a test can reason about +// exactly one contract. +func oneContractConfig(chain *mockChain, scenario string) *config.LoadConfig { + return &config.LoadConfig{ + ChainID: testChainID, + GenesisHash: testGenesisHash, + Endpoints: []string{chain.url}, + Accounts: &config.AccountConfig{Accounts: 4}, + Scenarios: []config.Scenario{{Name: scenario, Weight: 1}}, + } +} + +// recordedAddress is the address the fixtures record. Nothing derives it; the +// mock chain simply serves code there. +var recordedAddress = common.HexToAddress("0x00000000000000000000000000000000000000AA") + +// TestRegistryHitSendsNoDeployment asserts AS-1.1 and SC-001: a recorded +// contract whose code verifies is bound, and no deployment is sent. +func TestRegistryHitSendsNoDeployment(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode)} + + deployer := types.NewAccount(false) + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + require.NoError(t, err) + + require.Zero(t, chain.txCount(), + "the run sent %d transactions against a chain whose contract is already recorded", + chain.txCount()) + + // AS-1.1: and it generates against the recorded address, not some other one. + for _, tx := range generateN(t, newTestRng(1), gen, 5) { + require.Equal(t, recordedAddress, *tx.EthTx.To()) + } +} + +// TestRegistryHitBreaksWhenTheAddressIsWrong is the control for the test above. +// A no-deployment assertion is worthless unless it fails when the fixture stops +// matching, so this asserts the failure directly. +func TestRegistryHitBreaksWhenTheAddressIsWrong(t *testing.T) { + elsewhere := common.HexToAddress("0x00000000000000000000000000000000000000BB") + chain := newMockChain(t, mockChainConfig{ + // The chain serves the code at the recorded address, but the file names + // a different one, which holds nothing. + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{chainFileFor(t, scenarios.StorageRW, elsewhere, recordedCode)} + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, types.NewAccount(false)) + require.Error(t, err, "a recorded address holding no code started the run") + require.ErrorContains(t, err, "holds no code") +} + +// TestStaleEntryFailsBeforeAnyTransaction asserts AS-3.1, AS-3.3 and SC-003: a +// mismatch stops the run at startup, names what a reader needs, and leaves the +// chain untouched. +func TestStaleEntryFailsBeforeAnyTransaction(t *testing.T) { + cases := []struct { + name string + served []byte + wantPhrase string + }{ + {"the address holds no code", nil, "holds no code"}, + {"the address holds different code", []byte{0xfe, 0xfe}, "different code than recorded"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: tc.served}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{ + chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode), + } + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err) + require.ErrorContains(t, err, tc.wantPhrase) + + // AS-3.1: the message names the chain, the contract, and the address. + require.ErrorContains(t, err, testChainName) + require.ErrorContains(t, err, scenarios.StorageRW) + require.ErrorContains(t, err, recordedAddress.String()) + + // AS-3.3, CDR-009: nothing was deployed, and nothing was sent. + // Redeploying over a stale entry looks like a fix and is not: it + // hides that the registry no longer describes the chain. + require.Zero(t, chain.txCount(), + "the run sent %d transactions after a mismatch", chain.txCount()) + }) + } +} + +// TestMissingEntryDeploysAndRecords asserts AS-2.1, AS-2.2 and SC-002: no entry +// means deploy, and the run writes a reviewable chain file describing what it +// deployed. +func TestMissingEntryDeploysAndRecords(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + cfg := oneContractConfig(chain, scenarios.StorageRW) + recordPath := filepath.Join(t.TempDir(), "deployed.json") + cfg.ChainRecordPath = recordPath + cfg.SeiChainID = testChainName + + deployer := types.NewAccount(false) + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + require.NoError(t, err) + + // AS-2.1: it deployed. + require.Equal(t, 1, chain.txCount(), "the run deployed a different number of contracts") + deployedAt := crypto.CreateAddress(deployer.Address, 0) + + // AS-2.2: and wrote what it deployed. + raw, err := os.ReadFile(recordPath) + require.NoError(t, err, "the run deployed and wrote no chain file") + + var written registry.Chain + require.NoError(t, json.Unmarshal(raw, &written)) + require.Equal(t, testChainID, written.ChainID) + require.Equal(t, testGenesisHash, written.GenesisHash) + require.Len(t, written.Contracts, 1) + require.Equal(t, scenarios.StorageRW, written.Contracts[0].Name) + require.Equal(t, deployedAt, written.Contracts[0].Address) + require.Equal(t, crypto.Keccak256Hash(defaultCode), written.Contracts[0].CodeHash, + "the recorded hash must be the code the chain serves, not the compiled bytecode") + + // The file it wrote is a file it can read: that is the operator workflow. + reloaded, err := registry.Load(recordPath) + require.NoError(t, err) + _, ok := reloaded.Chain(testChainID, testGenesisHash) + require.True(t, ok, "the written file does not match its own identity") +} + +// TestSuppliedFileBeatsTheBinary asserts AS-2.4 and CDR-020. The compiled-in +// registry ships empty, so this asserts the layering rule directly: the last +// file supplied wins for a chain two files both name. +func TestSuppliedFileBeatsTheBinary(t *testing.T) { + first := common.HexToAddress("0x00000000000000000000000000000000000000CC") + second := common.HexToAddress("0x00000000000000000000000000000000000000DD") + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{first: recordedCode, second: recordedCode}, + }) + + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{ + chainFileFor(t, scenarios.StorageRW, first, recordedCode), + chainFileFor(t, scenarios.StorageRW, second, recordedCode), + } + + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, types.NewAccount(false)) + require.NoError(t, err) + require.Zero(t, chain.txCount()) + + for _, tx := range generateN(t, newTestRng(1), gen, 5) { + require.Equal(t, second, *tx.EthTx.To(), "the later chain file did not win") + } +} + +// TestForceDeployIgnoresARecordedEntry asserts SC-005 and CDR-016. +func TestForceDeployIgnoresARecordedEntry(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode)} + cfg.Scenarios[0].ForceDeploy = true + + deployer := types.NewAccount(false) + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + require.NoError(t, err) + + require.Equal(t, 1, chain.txCount(), "forceDeploy did not deploy") + fresh := crypto.CreateAddress(deployer.Address, 0) + for _, tx := range generateN(t, newTestRng(1), gen, 5) { + require.Equal(t, fresh, *tx.EthTx.To(), + "forceDeploy deployed and then bound the recorded address anyway") + } +} + +// TestExplicitAddressBindsAndDeploysNothing asserts CDR-004: a contract deployed +// outside this repo binds without a registry entry and without a deployment. +func TestExplicitAddressBindsAndDeploysNothing(t *testing.T) { + outside := common.HexToAddress("0x00000000000000000000000000000000000000EE") + chain := newMockChain(t, mockChainConfig{}) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.Scenarios[0].ContractAddress = outside.Hex() + + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, types.NewAccount(false)) + require.NoError(t, err) + require.Zero(t, chain.txCount(), "an explicit address still deployed") + + for _, tx := range generateN(t, newTestRng(1), gen, 5) { + require.Equal(t, outside, *tx.EthTx.To()) + } +} + +// TestStartupReadsCodeOncePerContract asserts CDR-012 and SC-004: startup cost +// is fixed per contract, and does not move with the account count. +func TestStartupReadsCodeOncePerContract(t *testing.T) { + countFor := func(t *testing.T, accounts int) int { + t.Helper() + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.Accounts = &config.AccountConfig{Accounts: accounts} + cfg.ChainFiles = []string{ + chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode), + } + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, types.NewAccount(false)) + require.NoError(t, err) + return len(chain.codeReads()) + } + + few := countFor(t, 4) + many := countFor(t, 400) + require.Equal(t, few, many, + "startup issued %d code reads for 4 accounts and %d for 400, so the cost "+ + "scales with the account pool", few, many) + require.Equal(t, 1, few, "one contract should cost one code read, not %d", few) +} + +// TestUndrivenContractIsNeverRead asserts CDR-022: a run resolves only what its +// profile drives. +func TestUndrivenContractIsNeverRead(t *testing.T) { + other := common.HexToAddress("0x00000000000000000000000000000000000000FF") + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode, other: recordedCode}, + }) + + // A chain file naming two contracts, against a profile driving one. + path := filepath.Join(t.TempDir(), "two.json") + require.NoError(t, registry.WriteChain(path, registry.Chain{ + ChainID: testChainID, + ChainName: testChainName, + GenesisHash: testGenesisHash, + Contracts: []registry.Contract{ + {Name: scenarios.StorageRW, Address: recordedAddress, + CodeHash: crypto.Keccak256Hash(recordedCode)}, + {Name: scenarios.ERC20, Address: other, + CodeHash: crypto.Keccak256Hash(recordedCode)}, + }, + })) + + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.ChainFiles = []string{path} + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, types.NewAccount(false)) + require.NoError(t, err) + + require.Equal(t, []common.Address{recordedAddress}, chain.codeReads(), + "the run read a contract no scenario in its profile drives") +} + +// TestTwoRunsOnOneChainDeployTheirOwn asserts CDR-013. Two runs against one +// ephemeral chain must each drive their own contract: sharing one would put +// contention in neither profile, so both would measure a workload nobody +// configured. +func TestTwoRunsOnOneChainDeployTheirOwn(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + + addresses := make([]common.Address, 0, 2) + for range 2 { + deployer := types.NewAccount(false) + cfg := oneContractConfig(chain, scenarios.StorageRW) + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + require.NoError(t, err) + + txs := generateN(t, newTestRng(1), gen, 3) + require.NotEmpty(t, txs) + addresses = append(addresses, *txs[0].EthTx.To()) + } + + require.NotEqual(t, addresses[0], addresses[1], + "both runs drove the contract at %s, so each measured contention the "+ + "other created", addresses[0]) +} diff --git a/generator/scenarios/Disperse.go b/generator/scenarios/Disperse.go index 8701d9e..d29dcca 100644 --- a/generator/scenarios/Disperse.go +++ b/generator/scenarios/Disperse.go @@ -55,26 +55,6 @@ func (s *DisperseScenario) SetContract(contract *bindings.Disperse) { s.contract = contract } -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *DisperseScenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewDisperse(address, client) - return err -} - // CreateContractTransaction implements ContractDeployer interface - creates Disperse transaction func (s *DisperseScenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { // create new accounts so that it auto-creates the accounts. diff --git a/generator/scenarios/ERC20.go b/generator/scenarios/ERC20.go index 1e69faf..2482fda 100644 --- a/generator/scenarios/ERC20.go +++ b/generator/scenarios/ERC20.go @@ -54,26 +54,6 @@ func (s *ERC20Scenario) DeployContract(opts *bind.TransactOpts, client *ethclien return address, tx, err } -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *ERC20Scenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewERC20(address, client) - return err -} - // CreateContractTransaction implements ContractDeployer interface - creates ERC20 transaction func (s *ERC20Scenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { auth.GasLimit = 72156 diff --git a/generator/scenarios/ERC20Conflict.go b/generator/scenarios/ERC20Conflict.go index aba3f69..99d5fea 100644 --- a/generator/scenarios/ERC20Conflict.go +++ b/generator/scenarios/ERC20Conflict.go @@ -54,26 +54,6 @@ func (s *ERC20ConflictScenario) SetContract(contract *bindings.ERC20Conflict) { s.contract = contract } -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *ERC20ConflictScenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewERC20Conflict(address, client) - return err -} - // CreateContractTransaction implements ContractDeployer interface - creates ERC20Conflict transaction func (s *ERC20ConflictScenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { auth.GasLimit = 22460 diff --git a/generator/scenarios/ERC20Noop.go b/generator/scenarios/ERC20Noop.go index 8b46035..cd72612 100644 --- a/generator/scenarios/ERC20Noop.go +++ b/generator/scenarios/ERC20Noop.go @@ -54,26 +54,6 @@ func (s *ERC20NoopScenario) SetContract(contract *bindings.ERC20Noop) { s.contract = contract } -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *ERC20NoopScenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewERC20Noop(address, client) - return err -} - // CreateContractTransaction implements ContractDeployer interface - creates ERC20Noop transaction func (s *ERC20NoopScenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { auth.GasLimit = 22460 diff --git a/generator/scenarios/ERC721.go b/generator/scenarios/ERC721.go index ce3c07d..764987a 100644 --- a/generator/scenarios/ERC721.go +++ b/generator/scenarios/ERC721.go @@ -62,23 +62,3 @@ func (s *ERC721Scenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.T auth.GasLimit = 22460 return s.contract.Mint(auth, scenario.Receiver, big.NewInt(atomic.AddInt64(&s.id, 1))) } - -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *ERC721Scenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewERC721(address, client) - return err -} diff --git a/generator/scenarios/StorageRW.go b/generator/scenarios/StorageRW.go index 5b9c316..d97b299 100644 --- a/generator/scenarios/StorageRW.go +++ b/generator/scenarios/StorageRW.go @@ -93,34 +93,6 @@ func (s *StorageRWScenario) SetContract(contract *bindings.StorageRWv1) { s.contract = contract } -// Attach implements TxGenerator interface - attaches to an existing contract -func (s *StorageRWScenario) Attach(config *config.LoadConfig, address common.Address) error { - // Call base Attach to set deployed flag and config - if err := s.ContractScenarioBase.Attach(config, address); err != nil { - return err - } - - var client *ethclient.Client - var err error - if !config.MockDeploy { - client, err = ethclient.Dial(config.Endpoints[0]) - if err != nil { - return err - } - } - - s.contract, err = bindings.NewStorageRWv1(address, client) - return err -} - -// CreateContractTransaction implements ContractDeployer interface - builds one -// StorageRWv1 transaction whose slot (key contention), calldata pad (tx size), -// and operation are drawn from the scenario config. With none of the three -// configured it falls back to a single-slot empty-pad rmw and draws no -// randomness. See package doc for the gas rationale. -// -// The draws run in a fixed order: slot, then pad, then operation. That order -// must stay stable — all three share the run's single PRNG, so reordering them // shifts every subsequent draw and diverges a replay at the same seed. func (s *StorageRWScenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { slot, err := s.pickSlot(rng) diff --git a/generator/scenarios/StorageRW_test.go b/generator/scenarios/StorageRW_test.go index 6cb9bb6..83d9e77 100644 --- a/generator/scenarios/StorageRW_test.go +++ b/generator/scenarios/StorageRW_test.go @@ -46,9 +46,10 @@ func TestStorageRWDeployAndGenerate(t *testing.T) { gen := scenarios.CreateScenario(config.Scenario{Name: scenarios.StorageRW}) - // Mirror generator.mockDeployAll: attach the bound contract at a known address. + // Mirror generator.mockPrepareAll: mark ready, then bind at a known address. contractAddr := types.GenerateAccounts(1, false)[0].Address - require.NoError(t, gen.Attach(cfg, contractAddr)) + require.NoError(t, gen.Ready(cfg)) + require.NoError(t, gen.Binder()(nil, contractAddr)) // Build the tx scenario the way the weighted generator does: a funded sender. sender := types.GenerateAccounts(1, true)[0] @@ -99,7 +100,8 @@ func newAttachedStorageRW(t *testing.T, sc config.Scenario) (scenarios.TxGenerat Endpoints: []string{"http://localhost:8545"}, } gen := scenarios.CreateScenario(sc) - require.NoError(t, gen.Attach(cfg, types.GenerateAccounts(1, false)[0].Address)) + require.NoError(t, gen.Ready(cfg)) + require.NoError(t, gen.Binder()(nil, types.GenerateAccounts(1, false)[0].Address)) return gen, &types.TxScenario{ Name: scenarios.StorageRW, Nonce: 0, diff --git a/generator/scenarios/base.go b/generator/scenarios/base.go index 5efed74..fa2d04f 100644 --- a/generator/scenarios/base.go +++ b/generator/scenarios/base.go @@ -36,10 +36,25 @@ type TxGenerator interface { // transaction reaches the metrics without the dimension. Operation() string Generate(rng *mrand.Rand, scenario *types.TxScenario) (*ethtypes.Transaction, error) - Attach(config *config.LoadConfig, address common.Address) error + // Ready marks the scenario able to generate, and records the config it + // generates against. It takes no address: an address is a fact about + // deployment, and a scenario's job is to shape a transaction. + Ready(config *config.LoadConfig) error + // Binder returns the hand-off a preparation step drives to give this + // scenario its contract, or nil for a scenario that drives none. The step + // supplies the backend and the address, so no scenario opens a connection. + Binder() ContractBinder Deploy(ctx context.Context, config *config.LoadConfig, deployer types.Account) (common.Address, error) } +// ContractBinder binds one contract and hands the instance to the scenario that +// drives it. A scenario builds one at construction, where its contract type is +// known, so a preparation step can drive it without knowing that type. +// +// The step owns the backend and the address. That is what keeps a scenario from +// dialing its own client and from holding an address as state. +type ContractBinder func(backend bind.ContractBackend, address common.Address) error + // ScenarioDeployer defines the interface for scenario-specific deployment logic // This can be implemented by both contract and non-contract scenarios type ScenarioDeployer interface { @@ -49,9 +64,6 @@ type ScenarioDeployer interface { // For non-contracts: performs any initialization and returns zero address. DeployScenario(ctx context.Context, config *config.LoadConfig, deployer types.Account) (common.Address, error) - // AttachScenario connects to an existing contract. - AttachScenario(config *config.LoadConfig, address common.Address) common.Address - // CreateTransaction creates a transaction for this scenario CreateTransaction(rng *mrand.Rand, config *config.LoadConfig, scenario *types.TxScenario) (*ethtypes.Transaction, error) } @@ -77,11 +89,13 @@ type ContractDeployer[T any] interface { CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) } -// ScenarioBase provides common functionality for all scenarios +// ScenarioBase holds no contract address. CDR-021 keeps an address out of a +// scenario: an address is a fact about deployment, and a scenario's job is to +// shape a transaction. The preparation step holds it, binds with it, and hands +// over the bound instance. type ScenarioBase struct { config *config.LoadConfig deployed bool - address common.Address deployer ScenarioDeployer scenarioConfig config.Scenario @@ -104,19 +118,22 @@ func (s *ScenarioBase) Deploy(ctx context.Context, config *config.LoadConfig, de if err != nil { return common.Address{}, err } - s.address = address s.deployed = true - return s.address, nil + return address, nil } -// Attach connects to an existing contract. -func (s *ScenarioBase) Attach(config *config.LoadConfig, address common.Address) error { +// Ready marks the scenario able to generate, against the config supplied. A +// scenario that drives a contract is bound separately, through Binder. +func (s *ScenarioBase) Ready(config *config.LoadConfig) error { s.config = config - s.address = s.deployer.AttachScenario(config, address) s.deployed = true return nil } +// Binder reports that this scenario drives no contract. A contract scenario +// overrides it. +func (s *ScenarioBase) Binder() ContractBinder { return nil } + // Generate handles the common transaction generation flow func (s *ScenarioBase) Generate(rng *mrand.Rand, scenario *types.TxScenario) (*ethtypes.Transaction, error) { if !s.deployed { @@ -131,11 +148,6 @@ func (s *ScenarioBase) GetConfig() *config.LoadConfig { return s.config } -// GetAddress returns the deployed contract address (zero address for non-contract scenarios) -func (s *ScenarioBase) GetAddress() common.Address { - return s.address -} - // ContractScenarioBase provides common functionality for contract scenarios type ContractScenarioBase[T any] struct { *ScenarioBase @@ -156,23 +168,22 @@ func dial(config *config.LoadConfig) (*ethclient.Client, error) { return ethclient.Dial(config.Endpoints[0]) } -// AttachScenario implements AttachScenario interface for contract scenarios -func (c *ContractScenarioBase[T]) AttachScenario(config *config.LoadConfig, address common.Address) common.Address { - client, err := dial(config) - if err != nil { - panic("Failed to connect to Ethereum client: " + err.Error()) - } - - // Bind contract instance using the provided bind function - bindFunc := c.deployer.GetBindFunc() - contract, err := bindFunc(address, client) - if err != nil { - panic("Failed to bind contract: " + err.Error()) +// Binder returns the hand-off that binds this scenario's contract and stores the +// instance. The preparation step supplies its own backend and the address it +// resolved, so this scenario opens no connection and keeps no address. +// +// It replaces AttachScenario, which dialed a client of its own and panicked when +// either the dial or the bind failed. A startup failure now names the contract +// that could not bind instead of unwinding the process. +func (c *ContractScenarioBase[T]) Binder() ContractBinder { + return func(backend bind.ContractBackend, address common.Address) error { + contract, err := c.deployer.GetBindFunc()(address, backend) + if err != nil { + return fmt.Errorf("bind contract at %s: %w", address, err) + } + c.deployer.SetContract(contract) + return nil } - - // Store the contract instance - c.deployer.SetContract(contract) - return address } // deployTimeout bounds one deployment end to end: the nonce fetch, the send, and diff --git a/main.go b/main.go index eaad78d..35f02bc 100644 --- a/main.go +++ b/main.go @@ -67,6 +67,8 @@ func init() { rootCmd.Flags().String("metricsListenAddr", "0.0.0.0:9090", "The ip:port on which to export prometheus metrics.") rootCmd.Flags().Bool("ramp-up", false, "Ramp up loadtest") rootCmd.Flags().String("report-path", "", "Path to save the report") + rootCmd.Flags().StringArray("chain-file", nil, "Contract registry file describing the target chain and its deployed contracts. Repeatable; each layers over the registry compiled into the binary, and a later file wins.") + rootCmd.Flags().String("chain-record-path", "", "Where to write a chain file describing what this run deployed, for an operator to review and commit") rootCmd.Flags().String("txs-dir", "", "Path to save the transactions") rootCmd.Flags().Uint64("target-gas", 10_000_000, "Target gas per block") rootCmd.Flags().Int("num-blocks-to-write", 100, "Number of blocks to write") @@ -127,6 +129,15 @@ func runLoadTest(ctx context.Context, cmd *cobra.Command) error { cfg.MockDeploy = true } + // A --chain-file layers over what the profile already named, so a + // deployment can add a chain without rewriting the profile it ships. + if chainFiles, err := cmd.Flags().GetStringArray("chain-file"); err == nil && len(chainFiles) > 0 { + cfg.ChainFiles = append(cfg.ChainFiles, chainFiles...) + } + if recordPath, err := cmd.Flags().GetString("chain-record-path"); err == nil && recordPath != "" { + cfg.ChainRecordPath = recordPath + } + log.Printf("🚀 Starting Sei Chain Load Test v2") log.Printf("📁 Config file: %s", configFile) log.Printf("🎯 Endpoints: %d", len(cfg.Endpoints)) diff --git a/registry/chains_test.go b/registry/chains_test.go new file mode 100644 index 0000000..87e957e --- /dev/null +++ b/registry/chains_test.go @@ -0,0 +1,49 @@ +package registry_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-load/registry" +) + +// TestCommittedChainFilesParse is the gate on the compiled-in registry. Every +// file in registry/chains/ must parse and hold each field the format names. +// +// An unparseable committed entry fails every run that reads it, and it would +// otherwise reach main unnoticed: the directory ships empty, so no other test +// exercises a real file. This runs in CI as part of the package's tests. +func TestCommittedChainFilesParse(t *testing.T) { + entries, err := os.ReadDir("chains") + if err != nil { + t.Fatalf("read the compiled-in chains directory: %v", err) + } + + var found int + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") { + continue + } + found++ + path := filepath.Join("chains", entry.Name()) + t.Run(entry.Name(), func(t *testing.T) { + // Load applies the same parse and the same validation a run applies, + // so this gate cannot drift from what a run accepts. + r, err := registry.Load(path) + if err != nil { + t.Fatalf("%s does not parse, so it would fail every run that "+ + "reads it: %v", path, err) + } + if len(r.Sources()) == 0 { + t.Fatalf("%s parsed and produced no chain", path) + } + }) + } + + if found == 0 { + t.Log("registry/chains/ holds no chain files yet. This gate becomes " + + "load-bearing when the first one is committed.") + } +} From 3567773b0d8505af9587641316dd3f0c0db50ada Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 10:54:29 -0700 Subject: [PATCH 4/9] fix(registry): close the defects four blinded reviewers found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An xreview slate of four independent reviewers dissented. The systems lens reproduced seven defects with real tests; the platform lens found a cross-cutting one. This closes the ship-blocking set. Each fix carries a test that fails without it. AS-3.3 was false for any profile of more than one scenario. Resolution and deployment ran interleaved per instance, so a stale entry on the second scenario left the first one's contract deployed, paid for, and recorded nowhere — the litter this feature exists to remove. Reproduced, then fixed: prepareAll now runs three passes, and every address is decided and every recorded address verified before anything deploys. Startup had no timeout. ethclient.Dial over HTTP performs no I/O and its client sets no deadline, so an endpoint that accepts and never answers held startup open with nothing logged. Both chain-reading phases now run inside WithinBudget, which the deploy path already used. Two paths failed open, which is the one behaviour that cannot be right here. A run naming no genesisHash matched nothing and redeployed on every restart in silence; it now fails when the registry describes that chain id. An explicit contractAddress skipped verification entirely, so a typo produced a full run of green metrics against an address holding nothing; it now checks for code. The registry key was the raw profile string while every other name lookup in sei-load lowercases. Profiles write "ERC20Conflict"; the constants are lowercase. My test passed only because it keyed off the constant, which no real profile writes. The frozen file format admitted duplicate contract names, which the generator could itself produce from a profile naming one scenario twice. The later entry was unreachable forever. validate now rejects it, before any file is committed and the format is load-bearing. TestTwoRunsOnOneChainDeployTheirOwn certified CDR-013, which does not hold. It minted a fresh key per run; funder.Deployer hands every pod the same funding root. Renamed to say what it proves, and the real limitation is now in the package doc: concurrent runs on one key produce identical deployments. No test asserts that — reproducing it needs a race, and a test that asserts a bad property by winning a race is worse than the gap. Also: three orphaned AttachScenario methods survived on the EVMTransfer family, so SC-006's verifier — grep for the removed method — returned seven hits. Both package docs still described deployAll and Attach. A regex deletion had truncated CreateContractTransaction's doc comment in StorageRW.go, taking the frozen draw-order invariant with it; no verifier caught that, because none of them read comments. recordDeployments dialed a second client against CDR-023. The two new config fields skipped Scenario.Validate, so contractAddress and forceDeploy set together were accepted and one silently won. Verifiers: gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing. SC-006's grep now returns nothing. Known and not fixed, recorded in the review notes: chain-record-path cannot write in either deployed pod shape except /dev/stdout; symlinks walk past checkWritePath; the code hash cannot distinguish two instances of one contract; startup emits no span and the run summary carries no contract address. Co-Authored-By: Claude Opus 5 (1M context) --- config/config.go | 14 ++ generator/doc.go | 46 +++- generator/generator.go | 6 +- generator/prepare.go | 326 ++++++++++++++++--------- generator/registry_test.go | 127 +++++++++- generator/scenarios/EVMTransfer.go | 7 - generator/scenarios/EVMTransferFast.go | 7 - generator/scenarios/EVMTransferNoop.go | 7 - generator/scenarios/StorageRW.go | 8 + generator/scenarios/base.go | 4 - generator/scenarios/doc.go | 21 +- registry/registry.go | 24 ++ registry/resolve.go | 19 ++ 13 files changed, 442 insertions(+), 174 deletions(-) diff --git a/config/config.go b/config/config.go index a0735c3..ed195bd 100644 --- a/config/config.go +++ b/config/config.go @@ -8,6 +8,8 @@ import ( "io" "math/big" "time" + + "github.com/ethereum/go-ethereum/common" ) // LoadConfig stores the configuration for load-related settings. @@ -248,6 +250,18 @@ func (s *Scenario) Validate() error { if s.SizeDistribution == nil && len(s.SizeBuckets) != 0 { return fmt.Errorf("scenario %q: sizeBuckets has %d entries but no sizeDistribution samples them", s.Name, len(s.SizeBuckets)) } + // Contract selection is the same class of hazard: two ways of naming a + // contract, set together, silently pick one. An operator who set forceDeploy + // expecting a fresh contract would get the configured address instead, and + // measure the wrong thing without being told. + if s.ContractAddress != "" && s.ForceDeploy { + return fmt.Errorf("scenario %q: contractAddress and forceDeploy are both set, "+ + "but a run can only do one of bind that address and deploy a fresh contract", s.Name) + } + if s.ContractAddress != "" && !common.IsHexAddress(s.ContractAddress) { + return fmt.Errorf("scenario %q: contractAddress %q is not an address", + s.Name, s.ContractAddress) + } return s.Operations.validate(s.Name, operationsFor(s.Name)) } diff --git a/generator/doc.go b/generator/doc.go index 408719e..ca20543 100644 --- a/generator/doc.go +++ b/generator/doc.go @@ -6,17 +6,28 @@ // // 1. createScenarios — one scenario instance per config entry, each bound to an // account pool (its own, or the shared top-level pool). -// 2. deployAll — deploy the contract each instance needs, in sequence. +// 2. prepareAll — give each instance the contract it drives, from the registry +// where one is recorded and by deploying where none is. It also writes the +// chain file when the run is configured to record what it deployed, which is +// the one step that touches the operator's filesystem. // 3. build — expand the instances by weight and shuffle them into the // round-robin the run draws from. // -// An error in any step fails the run. A generator that cannot deploy has nothing -// valid to generate, so a failed deployment surfaces as a startup error rather -// than as a run that sends transactions to an address holding no contract. +// An error in any step fails the run. A generator that cannot resolve or deploy +// has nothing valid to generate, so the failure surfaces as a startup error +// rather than as a run that sends transactions to an address holding no +// contract. +// +// # Resolution before deployment +// +// prepareAll decides every address and verifies every recorded one before it +// deploys anything. The ordering is the contract: a stale entry on the last +// scenario must not leave a contract from the first one on the chain, paid for +// and recorded nowhere. See prepare.go. // // # The deployer is received, not minted // -// deployAll signs its deployments with the account NewGenerator is handed. +// prepareAll signs its deployments with the account NewGenerator is handed. // Paying for a deployment is a funding concern, and the funder package owns the // run's funded identity, so funder.Deployer names the account and this package // spends it. Minting a key here cannot work: no account pool holds it, so @@ -26,17 +37,34 @@ // # Deployment nonces // // A deployment leaves its nonce unset, so go-ethereum reads the deployer's -// pending nonce from the chain, and deployAll waits for the receipt before it -// sends the next one. This is what makes a deployer with on-chain history safe: +// pending nonce from the chain, and deployPlanned waits for the receipt before +// it sends the next one. This is what makes a deployer with on-chain history safe: // the funding root has spent nonces before the run, and spends more right after // these deployments when it funds the pool. A nonce derived from the instance // index is correct only for a key that starts at zero. Deploying concurrently // reintroduces the collision the sequence prevents; the funder package doc makes // the same argument for the same key. // +// # Per-run contract isolation is not guaranteed +// +// Two runs against one chain each deploy their own contract only when they hold +// different deployer keys. A creation address derives from the sender and its +// nonce, and every other input to a deployment here is a constant: the gas caps, +// the gas limit, and the constructor arguments. +// +// funder.Deployer hands every pod in a release the same funding root account. Two +// pods starting together therefore read the same pending nonce and produce +// byte-identical deployment transactions, so they bind one contract and contend +// on its storage. That contention is in neither profile, so both runs measure a +// workload nobody configured. +// +// Sequential runs on one key are safe, because the second reads a nonce the first +// advanced. Concurrent runs are not. Give each concurrent run its own deployer +// key until this is fixed. +// // # Mock deploy // -// Under config.MockDeploy no deployment reaches a chain. Each instance attaches -// its binding at a random address, which is enough to shape calldata, and the +// Under config.MockDeploy no deployment reaches a chain. Each instance binds at +// a random address against a nil backend, which is enough to shape calldata, and the // deployer goes unused. This is the path --dry-run and the unit tests take. package generator diff --git a/generator/generator.go b/generator/generator.go index ef6c927..bd3ed52 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -236,13 +236,9 @@ func NewGenerator(ctx context.Context, rng *mrand.Rand, cfg *config.LoadConfig, } // Step 2: give every scenario the contract it drives - prepared, err := b.prepareAll(ctx, deployer) - if err != nil { + if _, err := b.prepareAll(ctx, deployer); err != nil { return nil, fmt.Errorf("failed to prepare scenarios: %w", err) } - if err := b.recordDeployments(ctx, prepared); err != nil { - return nil, fmt.Errorf("failed to record what was deployed: %w", err) - } // Step 3: Create weighted scenarioGenerator g, err := b.build(rng) diff --git a/generator/prepare.go b/generator/prepare.go index bf5853a..e2b15ab 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -5,17 +5,38 @@ import ( "errors" "fmt" "log" + "strings" + "time" - "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" "github.com/sei-protocol/sei-load/registry" "github.com/sei-protocol/sei-load/types" + loadutils "github.com/sei-protocol/sei-load/utils" ) +// resolveTimeout bounds every chain read the preparation step makes: the code +// reads that verify recorded addresses, and the reads that record fresh ones. +// +// Without it the step inherits the run context, which carries a deadline only +// when --duration is set. An endpoint that accepts a connection and never +// answers would hold startup open with nothing logged. ethclient.Dial over HTTP +// performs no I/O and its client sets no timeout, so the dial cannot catch that +// case and this budget is the only thing that does. +const resolveTimeout = 60 * time.Second + +// plan is what one scenario instance will bind, decided before anything deploys. +type plan struct { + instance *scenarioInstance + // address is the address to bind, or zero when this instance must deploy. + address common.Address + // deploy is true when the registry named nothing and the run must create it. + deploy bool +} + // prepared reports what one scenario instance ended up bound to, so the caller -// can record a deployment and log which source supplied an address. +// can record a fresh deployment. type prepared struct { instance *scenarioInstance address common.Address @@ -27,23 +48,29 @@ type prepared struct { // prepareAll gives every scenario instance the contract it drives, and reports // what it deployed. // -// One client serves the whole step. It reads code, and it binds. It is not the -// client that sends load: CreateTransactionOpts sets auth.NoSend, so a bound -// contract hands the transaction back rather than sending it, and the sender's -// own per-endpoint clients send. +// It runs in three passes, and the order is the point. Every address is decided +// and every recorded address verified before anything deploys, so a stale entry +// on the last scenario cannot leave a contract from the first one on the chain, +// paid for and recorded nowhere. AS-3.3 requires that a failed run deployed +// nothing, and only the ordering makes it true for a profile of more than one. // -// The step resolves only the contracts the profile drives. A contract in the -// registry that no scenario needs is never read and never verified. +// One client serves all three passes and the recording that follows. It reads +// code, and it binds. It is not the client that sends load: CreateTransactionOpts +// sets auth.NoSend, so a bound contract hands the transaction back rather than +// sending it, and the sender's own per-endpoint clients send. func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) ([]prepared, error) { if g.config.MockDeploy { return nil, g.mockPrepareAll() } + if len(g.config.Endpoints) == 0 { + return nil, errors.New("no endpoints configured, so no contract can be resolved") + } reg, err := registry.Load(g.config.ChainFiles...) if err != nil { return nil, fmt.Errorf("load the contract registry: %w", err) } - logRegistrySources(reg) + logRegistrySource(reg, g.config.ChainID, g.config.GenesisHash) client, err := ethclient.Dial(g.config.Endpoints[0]) if err != nil { @@ -51,120 +78,184 @@ func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Accoun } defer client.Close() - results := make([]prepared, 0, len(g.instances)) - for _, instance := range g.instances { - result, err := g.prepareOne(ctx, reg, client, instance, deployer) - if err != nil { - return nil, fmt.Errorf("prepare %s: %w", instance.Name, err) - } - results = append(results, result) - } - return results, nil -} - -// prepareOne resolves, deploys if needed, and binds one scenario instance. -// -// A scenario that drives no contract is marked ready and nothing else. That is -// what keeps a registry lookup from happening for an ETH transfer. -func (g *generatorBuilder) prepareOne(ctx context.Context, reg *registry.Registry, - client *ethclient.Client, instance *scenarioInstance, deployer types.Account, -) (prepared, error) { - binder := instance.Scenario.Binder() - if binder == nil { - return prepared{instance: instance}, instance.Scenario.Ready(g.config) + var plans []plan + err = loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", + func(ctx context.Context) error { + plans, err = g.planAll(ctx, reg, client) + return err + }) + if err != nil { + return nil, err } - address, deployed, err := g.addressFor(ctx, reg, client, instance, deployer) + results, err := g.deployPlanned(ctx, plans, deployer) if err != nil { - return prepared{}, err + return nil, err } - if err := instance.Scenario.Ready(g.config); err != nil { - return prepared{}, err + if err := g.bindAll(client, results); err != nil { + return nil, err } - if err := binder(client, address); err != nil { - return prepared{}, err + return results, g.recordDeployments(ctx, client, results) +} + +// planAll decides what every instance binds, and verifies every recorded address. +// It deploys nothing and sends nothing. +func (g *generatorBuilder) planAll(ctx context.Context, reg *registry.Registry, + client *ethclient.Client, +) ([]plan, error) { + plans := make([]plan, 0, len(g.instances)) + for _, instance := range g.instances { + if instance.Scenario.Binder() == nil { + plans = append(plans, plan{instance: instance}) + continue + } + p, err := g.planOne(ctx, reg, client, instance) + if err != nil { + return nil, fmt.Errorf("prepare %s: %w", instance.Name, err) + } + plans = append(plans, p) } - return prepared{instance: instance, address: address, deployed: deployed}, nil + return plans, nil } -// addressFor decides which address a contract scenario binds, in a fixed order -// of precedence: an explicit address from the profile, a forced deployment, a +// planOne decides which address one contract scenario binds, in a fixed order of +// precedence: an explicit address from the profile, a forced deployment, a // registry entry, then a deployment because nothing named one. -func (g *generatorBuilder) addressFor(ctx context.Context, reg *registry.Registry, - client *ethclient.Client, instance *scenarioInstance, deployer types.Account, -) (common.Address, bool, error) { +func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, + client *ethclient.Client, instance *scenarioInstance, +) (plan, error) { cfg := instance.Config if cfg.ContractAddress != "" { - if !common.IsHexAddress(cfg.ContractAddress) { - return common.Address{}, false, fmt.Errorf( - "contractAddress %q is not an address", cfg.ContractAddress) - } address := common.HexToAddress(cfg.ContractAddress) + // An explicit address skips the registry, and it must not skip the code + // check. A typo would otherwise produce a full run of green metrics + // against an address that holds nothing: the calls succeed at the EVM + // layer, do nothing, and sei-load reads inclusion rather than execution. + if err := registry.VerifyHasCode(ctx, client, address); err != nil { + return plan{}, fmt.Errorf("contractAddress %s: %w", address, err) + } log.Printf("📌 %s: binding the configured address %s", instance.Name, address) - return address, false, nil + return plan{instance: instance, address: address}, nil } if cfg.ForceDeploy { log.Printf("🔁 %s: forceDeploy is set, deploying rather than resolving", instance.Name) - address, err := g.deployOne(ctx, instance, deployer) - return address, true, err + return plan{instance: instance, deploy: true}, nil } + name := contractNameFor(instance) address, mustDeploy, err := registry.Resolve(ctx, client, reg, - g.config.ChainID, g.config.GenesisHash, contractNameFor(instance)) + g.config.ChainID, g.config.GenesisHash, name) if err != nil { - return common.Address{}, false, err + return plan{}, err } if !mustDeploy { log.Printf("📖 %s: bound the recorded address %s, no deployment sent", instance.Name, address) - return address, false, nil + return plan{instance: instance, address: address}, nil } - address, err = g.deployOne(ctx, instance, deployer) - return address, true, err + // A miss because the run named no genesis hash is a misconfiguration, not a + // new chain. Every valid entry carries one, so an empty hash matches nothing + // and would deploy on every restart in silence — the behaviour this feature + // exists to remove. Fail rather than degenerate. + if g.config.GenesisHash == "" && reg.HasChainID(g.config.ChainID) { + return plan{}, fmt.Errorf( + "the registry describes chain %d but this run names no genesisHash, "+ + "so it matches nothing and would deploy %s again on every restart. "+ + "Set genesisHash in the profile, or set forceDeploy on this scenario", + g.config.ChainID, name) + } + log.Printf("🆕 %s: no registry entry for %s on chain %d (genesisHash %s), deploying", + instance.Name, name, g.config.ChainID, describeHash(g.config.GenesisHash)) + return plan{instance: instance, deploy: true}, nil } -// deployOne deploys the contract one instance drives. It keeps the sequential -// deployment the package doc describes: one deployer key stays in one ordered -// nonce stream. -func (g *generatorBuilder) deployOne(ctx context.Context, instance *scenarioInstance, +// deployPlanned creates every contract the plan says is missing. Every recorded +// address has already verified by the time this runs. +// +// It stays sequential: each deployment reads its nonce from the chain and is +// mined before the next is sent, so one deployer key stays in one ordered nonce +// stream. Deploying concurrently reintroduces the nonce collision the sequence +// prevents. +func (g *generatorBuilder) deployPlanned(ctx context.Context, plans []plan, deployer types.Account, -) (common.Address, error) { - if deployer.PrivKey == nil { - return common.Address{}, errors.New( - "deployer has no private key (a live deployment must be signed)") - } - log.Printf("Deploying scenario %s", instance.Name) - address, err := instance.Scenario.Deploy(ctx, g.config, deployer) - if err != nil { - return common.Address{}, err +) ([]prepared, error) { + results := make([]prepared, 0, len(plans)) + for _, p := range plans { + if !p.deploy { + results = append(results, prepared{instance: p.instance, address: p.address}) + continue + } + if deployer.PrivKey == nil { + return nil, errors.New( + "deployer has no private key (a live deployment must be signed)") + } + log.Printf("Deploying scenario %s", p.instance.Name) + address, err := p.instance.Scenario.Deploy(ctx, g.config, deployer) + if err != nil { + return nil, fmt.Errorf("prepare %s: %w", p.instance.Name, err) + } + log.Printf("🚀 Deployed %s at address: %s", p.instance.Name, address) + results = append(results, prepared{ + instance: p.instance, address: address, deployed: true, + }) } - if address != (common.Address{}) { - log.Printf("🚀 Deployed %s at address: %s", instance.Name, address) + return results, nil +} + +// bindAll marks every scenario ready and hands each contract scenario its bound +// instance, using the one client the step already holds. +func (g *generatorBuilder) bindAll(client *ethclient.Client, results []prepared) error { + for _, result := range results { + if err := result.instance.Scenario.Ready(g.config); err != nil { + return err + } + binder := result.instance.Scenario.Binder() + if binder == nil { + continue + } + if err := binder(client, result.address); err != nil { + return fmt.Errorf("prepare %s: %w", result.instance.Name, err) + } } - return address, nil + return nil } -// contractNameFor is the key a scenario's contract is recorded under. The -// scenario's configured name is that key, so a profile and a chain file agree -// without a second vocabulary to keep in step. +// contractNameFor is the key a scenario's contract is recorded under. +// +// It lowercases the profile's configured name, matching how the scenario factory +// resolves that same name. Every profile in this repo writes CamelCase, and the +// scenario constants are lowercase, so keying on the raw string would record +// "ERC20Conflict" from one profile and miss it from another that wrote +// "erc20conflict". One vocabulary, one case. func contractNameFor(instance *scenarioInstance) string { - return instance.Config.Name + return strings.ToLower(instance.Config.Name) } -// logRegistrySources says which file supplied each chain the registry holds. A -// run against the wrong chain file is otherwise silent. -func logRegistrySources(reg *registry.Registry) { - sources := reg.Sources() - if len(sources) == 0 { - log.Printf("📖 contract registry: no chains, so every contract deploys") - return +// describeHash renders a genesis hash for a log line, naming the empty case +// rather than printing nothing. +func describeHash(hash string) string { + if hash == "" { + return "(empty)" } - for name, source := range sources { - log.Printf("📖 contract registry: %s from %s", name, source) + return hash +} + +// logRegistrySource says which chain this run matched, and which file supplied +// it. Listing every chain the registry holds would not answer the question an +// operator has, which is whether their own chain was found. +func logRegistrySource(reg *registry.Registry, chainID int64, genesisHash string) { + chain, ok := reg.Chain(chainID, genesisHash) + if !ok { + log.Printf("📖 contract registry: no entry for chain %d (genesisHash %s); "+ + "the registry holds %d chain(s)", + chainID, describeHash(genesisHash), len(reg.Sources())) + return } + log.Printf("📖 contract registry: matched %s (chain %d) from %s, %d contract(s)", + chain.ChainName, chainID, reg.Sources()[chain.ChainName], len(chain.Contracts)) } // mockPrepareAll marks every scenario ready and binds each contract against a @@ -176,8 +267,7 @@ func (g *generatorBuilder) mockPrepareAll() error { return err } if binder := instance.Scenario.Binder(); binder != nil { - addr := types.NewAccount(false).Address - if err := binder(nilBackend(), addr); err != nil { + if err := binder(nil, types.NewAccount(false).Address); err != nil { return err } } @@ -185,38 +275,24 @@ func (g *generatorBuilder) mockPrepareAll() error { return nil } -// nilBackend is the backend the mock path binds against. abigen stores it and -// touches it only when a call needs the chain, which the mock path never does. -func nilBackend() bind.ContractBackend { return nil } - // recordDeployments writes a chain file describing what this run deployed, for // an operator to review and commit. // -// It writes nothing when the run deployed nothing, and nothing when the profile -// names no path. The file is not committed automatically, and an ephemeral -// chain's file should not be committed at all: that chain disappears after the -// run, so an entry naming it could never verify again. -func (g *generatorBuilder) recordDeployments(ctx context.Context, results []prepared) error { - if g.config.MockDeploy || g.config.ChainRecordPath == "" { - return nil - } - - deployed := make([]prepared, 0, len(results)) - for _, result := range results { - if result.deployed { - deployed = append(deployed, result) - } - } - if len(deployed) == 0 { +// It writes nothing when the run deployed nothing, and nothing when no path is +// configured. In a pod the only writable path is /dev/stdout: both the nightly +// Job and the canary Deployments mount their volumes read-only and set +// readOnlyRootFilesystem. +// +// An ephemeral chain's file is not committed. That chain disappears after the +// run, so an entry naming it could never verify again, and every later run +// reading it would fail. +func (g *generatorBuilder) recordDeployments(ctx context.Context, + client *ethclient.Client, results []prepared, +) error { + if g.config.ChainRecordPath == "" { return nil } - client, err := ethclient.Dial(g.config.Endpoints[0]) - if err != nil { - return fmt.Errorf("dial %s: %w", g.config.Endpoints[0], err) - } - defer client.Close() - chain := registry.Chain{ ChainID: g.config.ChainID, ChainName: g.config.SeiChainID, @@ -225,13 +301,27 @@ func (g *generatorBuilder) recordDeployments(ctx context.Context, results []prep if chain.ChainName == "" { chain.ChainName = fmt.Sprintf("chain-%d", g.config.ChainID) } - for _, result := range deployed { - contract, err := registry.Record(ctx, client, - contractNameFor(result.instance), result.address) - if err != nil { - return err - } - chain.Contracts = append(chain.Contracts, contract) + + err := loadutils.WithinBudget(ctx, resolveTimeout, "recording deployments", + func(ctx context.Context) error { + for _, result := range results { + if !result.deployed { + continue + } + contract, err := registry.Record(ctx, client, + contractNameFor(result.instance), result.address) + if err != nil { + return err + } + chain.Contracts = append(chain.Contracts, contract) + } + return nil + }) + if err != nil { + return err + } + if len(chain.Contracts) == 0 { + return nil } if err := registry.WriteChain(g.config.ChainRecordPath, chain); err != nil { diff --git a/generator/registry_test.go b/generator/registry_test.go index 959ced7..d26019e 100644 --- a/generator/registry_test.go +++ b/generator/registry_test.go @@ -304,18 +304,32 @@ func TestUndrivenContractIsNeverRead(t *testing.T) { "the run read a contract no scenario in its profile drives") } -// TestTwoRunsOnOneChainDeployTheirOwn asserts CDR-013. Two runs against one -// ephemeral chain must each drive their own contract: sharing one would put -// contention in neither profile, so both would measure a workload nobody -// configured. -func TestTwoRunsOnOneChainDeployTheirOwn(t *testing.T) { +// TestTwoRunsWithDistinctKeysDeployTheirOwn asserts what the code actually +// guarantees, which is narrower than CDR-013 as written. +// +// Isolation comes from the deployer key, not from anything this package does. +// Two runs holding DIFFERENT keys deploy to different addresses, because a +// creation address is derived from the sender and its nonce. Two runs holding +// the SAME key produce byte-identical deployment transactions — same nonce, same +// constant gas, same empty constructor args — and therefore one contract. +// +// That matters because funder.Deployer hands every pod in a release the same +// funding root account. Sequential runs on one key still differ, because the +// second reads a nonce the first advanced. CONCURRENT runs on one key do not: +// both read pending nonce zero, and every other input to the deployment is a +// constant, so they produce byte-identical transactions and one contract. +// +// No test here asserts that. Reproducing it needs a race, and a test that +// asserts a bad property by winning a race is worse than the gap it documents. +// The limitation is stated in the package doc instead. +func TestTwoRunsWithDistinctKeysDeployTheirOwn(t *testing.T) { chain := newMockChain(t, mockChainConfig{}) addresses := make([]common.Address, 0, 2) for range 2 { - deployer := types.NewAccount(false) cfg := oneContractConfig(chain, scenarios.StorageRW) - gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) require.NoError(t, err) txs := generateN(t, newTestRng(1), gen, 3) @@ -324,6 +338,101 @@ func TestTwoRunsOnOneChainDeployTheirOwn(t *testing.T) { } require.NotEqual(t, addresses[0], addresses[1], - "both runs drove the contract at %s, so each measured contention the "+ - "other created", addresses[0]) + "two runs with distinct keys drove the contract at %s", addresses[0]) +} + +// TestMismatchAfterAnEarlierDeployLeavesNothing asserts AS-3.3 for a profile of +// more than one scenario, which is the case that exposes ordering. +// +// The first scenario has no entry and would deploy; the second has a stale one. +// Resolution runs to completion before anything deploys, so the mismatch stops +// the run with nothing on the chain. Interleaving the two would leave the first +// contract deployed, paid for, and recorded nowhere — the litter this feature +// exists to remove. +func TestMismatchAfterAnEarlierDeployLeavesNothing(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: nil}, + }) + path := filepath.Join(t.TempDir(), "chain.json") + require.NoError(t, registry.WriteChain(path, registry.Chain{ + ChainID: testChainID, ChainName: testChainName, GenesisHash: testGenesisHash, + Contracts: []registry.Contract{{ + Name: scenarios.StorageRW, Address: recordedAddress, + CodeHash: crypto.Keccak256Hash(recordedCode), + }}, + })) + + cfg := &config.LoadConfig{ + ChainID: testChainID, GenesisHash: testGenesisHash, + Endpoints: []string{chain.url}, + Accounts: &config.AccountConfig{Accounts: 4}, + Scenarios: []config.Scenario{ + {Name: scenarios.ERC20, Weight: 1}, + {Name: scenarios.StorageRW, Weight: 1}, + }, + ChainFiles: []string{path}, + } + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err) + require.ErrorContains(t, err, "holds no code") + require.Zero(t, chain.txCount(), + "AS-3.3: the run deployed %d contract(s) before the mismatch stopped it, "+ + "and recorded none of them", chain.txCount()) +} + +// TestMissingGenesisHashFailsRatherThanDeploying asserts the fail-closed rule for +// the one path that used to fail open. +// +// Every valid chain file carries a genesis hash, so a run that names none +// matches nothing. Deploying would look identical to a chain the registry does +// not describe, and would repeat on every restart in silence. +func TestMissingGenesisHashFailsRatherThanDeploying(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.GenesisHash = "" + cfg.ChainFiles = []string{chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode)} + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err, "a run with no genesisHash silently deployed over a recorded contract") + require.ErrorContains(t, err, "genesisHash") + require.Zero(t, chain.txCount()) +} + +// TestExplicitAddressIsVerified asserts CDR-004's escape hatch still checks that +// something is there. A typo would otherwise produce a full run of green metrics +// against an address that holds nothing. +func TestExplicitAddressIsVerified(t *testing.T) { + chain := newMockChain(t, mockChainConfig{code: map[common.Address][]byte{}}) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.Scenarios[0].ContractAddress = "0x000000000000000000000000000000000000dEaD" + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err, "the run bound an address holding no code and started") + require.ErrorContains(t, err, "holds no code") +} + +// TestContractNameIsCaseInsensitive asserts the registry key matches how the +// scenario factory resolves the same name. Every profile in this repo writes +// CamelCase; the scenario constants are lowercase. +func TestContractNameIsCaseInsensitive(t *testing.T) { + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{recordedAddress: recordedCode}, + }) + cfg := oneContractConfig(chain, "StorageRW") // as a real profile writes it + cfg.ChainFiles = []string{chainFileFor(t, scenarios.StorageRW, recordedAddress, recordedCode)} + + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.NoError(t, err) + require.Zero(t, chain.txCount(), + "a CamelCase profile name missed a lowercase registry entry and redeployed") + for _, tx := range generateN(t, newTestRng(1), gen, 3) { + require.Equal(t, recordedAddress, *tx.EthTx.To()) + } } diff --git a/generator/scenarios/EVMTransfer.go b/generator/scenarios/EVMTransfer.go index 3e96e25..5700762 100644 --- a/generator/scenarios/EVMTransfer.go +++ b/generator/scenarios/EVMTransfer.go @@ -50,13 +50,6 @@ func (s *EVMTransferScenario) DeployScenario(ctx context.Context, config *config return common.Address{}, nil } -// AttachScenario implements ScenarioDeployer interface - no attachment needed for ETH transfers. -func (s *EVMTransferScenario) AttachScenario(config *config.LoadConfig, address common.Address) common.Address { - // No attachment needed for simple ETH transfers - // Return zero address to indicate no contract deployment - return common.Address{} -} - // CreateTransaction implements ScenarioDeployer interface - creates ETH transfer transaction func (s *EVMTransferScenario) CreateTransaction(rng *mrand.Rand, config *config.LoadConfig, scenario *types2.TxScenario) (*ethtypes.Transaction, error) { // Create transaction with value transfer diff --git a/generator/scenarios/EVMTransferFast.go b/generator/scenarios/EVMTransferFast.go index 3c862fc..83aa504 100644 --- a/generator/scenarios/EVMTransferFast.go +++ b/generator/scenarios/EVMTransferFast.go @@ -43,13 +43,6 @@ func (s *EVMTransferFastScenario) DeployScenario(ctx context.Context, config *co return common.Address{}, nil } -// AttachScenario implements ScenarioDeployer interface - no attachment needed for ETH transfers. -func (s *EVMTransferFastScenario) AttachScenario(config *config.LoadConfig, address common.Address) common.Address { - // No attachment needed for simple ETH transfers - // Return zero address to indicate no contract deployment - return common.Address{} -} - // CreateTransaction EVMTransferFastScenario ScenarioDeployer interface - creates ETH transfer transaction func (s *EVMTransferFastScenario) CreateTransaction(rng *mrand.Rand, config *config.LoadConfig, scenario *types2.TxScenario) (*ethtypes.Transaction, error) { // Create transaction with value transfer diff --git a/generator/scenarios/EVMTransferNoop.go b/generator/scenarios/EVMTransferNoop.go index 9537bb1..4b5b026 100644 --- a/generator/scenarios/EVMTransferNoop.go +++ b/generator/scenarios/EVMTransferNoop.go @@ -42,13 +42,6 @@ func (s *EVMTransferNoopScenario) DeployScenario(ctx context.Context, config *co return common.Address{}, nil } -// AttachScenario implements ScenarioDeployer interface - no attachment needed for ETH transfers. -func (s *EVMTransferNoopScenario) AttachScenario(config *config.LoadConfig, address common.Address) common.Address { - // No attachment needed for simple ETH transfers - // Return zero address to indicate no contract deployment - return common.Address{} -} - // CreateTransaction implements ScenarioDeployer interface - creates ETH transfer transaction func (s *EVMTransferNoopScenario) CreateTransaction(rng *mrand.Rand, config *config.LoadConfig, scenario *types2.TxScenario) (*ethtypes.Transaction, error) { // Create transaction with value transfer diff --git a/generator/scenarios/StorageRW.go b/generator/scenarios/StorageRW.go index d97b299..c6ddb06 100644 --- a/generator/scenarios/StorageRW.go +++ b/generator/scenarios/StorageRW.go @@ -93,6 +93,14 @@ func (s *StorageRWScenario) SetContract(contract *bindings.StorageRWv1) { s.contract = contract } +// CreateContractTransaction implements ContractDeployer interface - builds one +// StorageRWv1 transaction whose slot (key contention), calldata pad (tx size), +// and operation are drawn from the scenario config. With none of the three +// configured it falls back to a single-slot empty-pad rmw and draws no +// randomness. See package doc for the gas rationale. +// +// The draws run in a fixed order: slot, then pad, then operation. That order +// must stay stable — all three share the run's single PRNG, so reordering them // shifts every subsequent draw and diverges a replay at the same seed. func (s *StorageRWScenario) CreateContractTransaction(rng *mrand.Rand, auth *bind.TransactOpts, scenario *types.TxScenario) (*ethtypes.Transaction, error) { slot, err := s.pickSlot(rng) diff --git a/generator/scenarios/base.go b/generator/scenarios/base.go index fa2d04f..071ef4e 100644 --- a/generator/scenarios/base.go +++ b/generator/scenarios/base.go @@ -171,10 +171,6 @@ func dial(config *config.LoadConfig) (*ethclient.Client, error) { // Binder returns the hand-off that binds this scenario's contract and stores the // instance. The preparation step supplies its own backend and the address it // resolved, so this scenario opens no connection and keeps no address. -// -// It replaces AttachScenario, which dialed a client of its own and panicked when -// either the dial or the bind failed. A startup failure now names the contract -// that could not bind instead of unwinding the process. func (c *ContractScenarioBase[T]) Binder() ContractBinder { return func(backend bind.ContractBackend, address common.Address) error { contract, err := c.deployer.GetBindFunc()(address, backend) diff --git a/generator/scenarios/doc.go b/generator/scenarios/doc.go index 962a84b..e4e4771 100644 --- a/generator/scenarios/doc.go +++ b/generator/scenarios/doc.go @@ -4,7 +4,8 @@ // // # The contract-scenario pattern // -// Every scenario satisfies TxGenerator (Name/Generate/Attach/Deploy). Non-contract +// Every scenario satisfies TxGenerator (Name/Operation/Generate/Ready/Binder/ +// Deploy). Non-contract // scenarios (the EVMTransfer family) implement it directly; contract scenarios // compose ContractScenarioBase[T], which factors out the deploy-wait-bind flow // and the per-tx auth construction so the concrete scenario only supplies its @@ -14,22 +15,26 @@ // binding) and implements ContractDeployer[T]: // // - DeployContract — deploy the contract for this run. -// - GetBindFunc — return the binding's constructor so the base can bind the -// deployed (or attached) address. +// - GetBindFunc — return the binding's constructor, so a caller holding no +// knowledge of T can bind the address it resolved. // - SetContract — receive the bound instance for later CreateContractTransaction // calls. // - CreateContractTransaction — build one load transaction against the contract. // // The base owns the rest: DeployScenario deploys, waits for the receipt, asserts -// success, then binds and hands back the instance via SetContract; AttachScenario -// binds an already-deployed address the same way; CreateTransaction builds the -// per-tx auth and delegates to CreateContractTransaction. +// success, then binds and hands back the instance via SetContract; Binder returns +// the closure a preparation step drives to bind an already-deployed address, with +// the step's own client; CreateTransaction builds the per-tx auth and delegates to +// CreateContractTransaction. +// +// A scenario never holds a contract address and never opens a connection. An +// address is a fact about deployment, and a scenario shapes transactions. // // # MockDeploy attach // -// Under config.MockDeploy a scenario attaches to a known address without a live +// Under config.MockDeploy a scenario binds a known address without a live // endpoint, so the bind backend is nil. This is the path the tests and -// generator.mockDeployAll exercise: bind at an address, produce calldata, but +// generator.mockPrepareAll exercise: bind at an address, produce calldata, but // never send. CreateContractTransaction must therefore stay pure (it shapes a // transaction; it does not touch the chain). // diff --git a/registry/registry.go b/registry/registry.go index 9d72083..cfb7ed1 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -153,10 +153,18 @@ func (c Chain) validate() error { if c.GenesisHash == "" { return errors.New("genesisHash is required") } + seen := make(map[string]int, len(c.Contracts)) for i, contract := range c.Contracts { if contract.Name == "" { return fmt.Errorf("contracts[%d]: name is required", i) } + if first, ok := seen[contract.Name]; ok { + return fmt.Errorf( + "contracts[%d] %s: already named at contracts[%d]. A lookup "+ + "returns the first match, so the later entry is unreachable", + i, contract.Name, first) + } + seen[contract.Name] = i if contract.Address == (common.Address{}) { return fmt.Errorf("contracts[%d] %s: address is required", i, contract.Name) @@ -176,6 +184,22 @@ func (r *Registry) Chain(chainID int64, genesisHash string) (Chain, bool) { return chain, ok } +// HasChainID reports whether the registry holds any chain with this EVM chain +// id, whatever its genesis hash. +// +// A caller uses it to tell two cases apart that a Chain miss collapses: the +// registry describes this chain and the run named the wrong genesis hash, or the +// registry does not describe this chain at all. The first is a misconfiguration +// and the second is an ordinary deploy. +func (r *Registry) HasChainID(chainID int64) bool { + for key := range r.chains { + if key.chainID == chainID { + return true + } + } + return false +} + // Sources reports which file supplied each chain, keyed by chain name, for the // run to log. A compiled-in chain reports "embedded". func (r *Registry) Sources() map[string]string { diff --git a/registry/resolve.go b/registry/resolve.go index 1076452..db756de 100644 --- a/registry/resolve.go +++ b/registry/resolve.go @@ -97,6 +97,25 @@ func codeHashAt(ctx context.Context, code CodeReader, addr common.Address) (comm return crypto.Keccak256Hash(runtime), nil } +// VerifyHasCode reports whether an address holds any contract code. It is the +// check for an address the registry does not describe, where there is no +// recorded hash to compare against. +// +// A call to an address holding no code succeeds at the EVM layer and does +// nothing, and a load generator that reads inclusion rather than execution +// cannot see that. This is the difference between an address nobody verified and +// a run that measures nothing. +func VerifyHasCode(ctx context.Context, code CodeReader, addr common.Address) error { + hash, err := codeHashAt(ctx, code, addr) + if err != nil { + return fmt.Errorf("read code at %s: %w", addr, err) + } + if hash == (common.Hash{}) { + return fmt.Errorf("%s holds no code", addr) + } + return nil +} + // Resolve returns the address for a named contract, and reports whether the // caller needs to deploy. It verifies before it returns an address. // From 84276b65b1ddb1d76f246a5c116462069a3075b4 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 11:31:56 -0700 Subject: [PATCH 5/9] refactor(registry): fail closed on an unrecordable run, and cut comment volume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the startup check for the one pairing that still failed late: chainRecordPath set with genesisHash empty deployed every contract and then failed on WriteChain's validation, leaving contracts on the chain that nothing recorded. ValidateRecording refuses it before the first dial. It runs in runLoadTest rather than loadConfig, because the flag that sets the field is merged after the profile is parsed. Then a brandon-code pass over the feature. prepareAll now reads as its step sequence — plan, deploy, bind, record — with the WithinBudget ceremony behind planAllWithinBudget. The closure had also been reassigning the orchestration's own err from inside itself, which worked and read badly. Comment volume came down against principles 13 and 14. An unexported function defaults to no comment and has to earn one: addEmbedded, describeHash and bindAll lost theirs, and the ones that hold an invariant a future editor could break silently kept theirs, trimmed. codeHashAt keeps its zero-hash rule because both error branches depend on it. deployPlanned keeps the nonce-ordering invariant. validate, decodeStrict and contractNameFor each keep the one sentence that says why. Verified placement with go doc -u rather than by reading the file: a group fused by a missing blank line renders under the wrong symbol and looks correct in source. No fused groups. checkWritePath led with a claim its own second paragraph withdrew — "enforceable rather than a convention" and "still cannot edit it" — and the review proved a symlink walks past it. It now leads with the two paths it actually matches and names the escape. chains/README.md had copied the strong form and dropped the caveat; it now carries the honest one, says bootstrapping is a local operation rather than a deployed one, and states the forward-compatibility cost of adding a field. Verifiers: gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing unchanged — which is the proof this changed structure and comments only. Co-Authored-By: Claude Opus 5 (1M context) --- config/config.go | 23 ++++++++++++++++ config/config_test.go | 32 ++++++++++++++++++++++ generator/prepare.go | 57 +++++++++++++++++++-------------------- main.go | 5 ++++ registry/chains/README.md | 28 +++++++++++++------ registry/registry.go | 17 +++++------- registry/resolve.go | 37 +++++++++++-------------- 7 files changed, 129 insertions(+), 70 deletions(-) diff --git a/config/config.go b/config/config.go index ed195bd..8d38adc 100644 --- a/config/config.go +++ b/config/config.go @@ -265,6 +265,29 @@ func (s *Scenario) Validate() error { return s.Operations.validate(s.Name, operationsFor(s.Name)) } +// ValidateRecording rejects a run that would deploy contracts and then fail to +// record them. +// +// WriteChain requires a genesis hash, because half a chain's identity does not +// identify it. Without this check that requirement is discovered at the end of +// startup, after every contract is deployed and paid for — so the run leaves +// contracts on the chain, records none of them, and exits non-zero. Refusing up +// front costs nothing and leaves the chain untouched. +// +// A caller MUST run this after merging command-line flags, because +// --chain-record-path sets the field this reads. +func (c *LoadConfig) ValidateRecording() error { + if c.ChainRecordPath == "" || c.GenesisHash != "" { + return nil + } + return fmt.Errorf( + "chainRecordPath is %q but genesisHash is empty: a chain file needs the "+ + "genesis hash to identify its chain, so the run would deploy every "+ + "contract and then fail to write the record. Set genesisHash, or "+ + "clear chainRecordPath", + c.ChainRecordPath) +} + // ValidateScenarios runs each scenario's Validate and names the scenario that // failed. loadConfig calls it after unmarshalling. func (c *LoadConfig) ValidateScenarios() error { diff --git a/config/config_test.go b/config/config_test.go index dea65f2..94cff25 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -244,3 +244,35 @@ func TestValidateRejectsUnknownOperation(t *testing.T) { require.NoError(t, err, "a map-typed field accepts the key at parse") require.ErrorContains(t, cfg.ValidateScenarios(), `unknown operation "reads"`) } + +// TestValidateRecording covers the pairing that would otherwise deploy every +// contract and then fail on the write. +func TestValidateRecording(t *testing.T) { + cases := []struct { + name string + recordPath string + genesisHash string + wantErr bool + }{ + {"neither set", "", "", false}, + {"recording with an identity", "/dev/stdout", "3f1a", false}, + {"an identity and no recording", "", "3f1a", false}, + {"recording with no identity", "/dev/stdout", "", true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &LoadConfig{ + ChainRecordPath: tc.recordPath, + GenesisHash: tc.genesisHash, + } + err := cfg.ValidateRecording() + if tc.wantErr { + require.Error(t, err, "the run would deploy and then fail to record") + require.ErrorContains(t, err, "genesisHash") + return + } + require.NoError(t, err) + }) + } +} diff --git a/generator/prepare.go b/generator/prepare.go index e2b15ab..4907df7 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -16,14 +16,11 @@ import ( loadutils "github.com/sei-protocol/sei-load/utils" ) -// resolveTimeout bounds every chain read the preparation step makes: the code -// reads that verify recorded addresses, and the reads that record fresh ones. +// resolveTimeout bounds every chain read the preparation step makes. // -// Without it the step inherits the run context, which carries a deadline only -// when --duration is set. An endpoint that accepts a connection and never -// answers would hold startup open with nothing logged. ethclient.Dial over HTTP -// performs no I/O and its client sets no timeout, so the dial cannot catch that -// case and this budget is the only thing that does. +// Nothing else does. ethclient.Dial over HTTP performs no I/O and its client +// sets no timeout, so an endpoint that accepts a connection and never answers +// would hold startup open with nothing logged. const resolveTimeout = 60 * time.Second // plan is what one scenario instance will bind, decided before anything deploys. @@ -78,12 +75,7 @@ func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Accoun } defer client.Close() - var plans []plan - err = loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", - func(ctx context.Context) error { - plans, err = g.planAll(ctx, reg, client) - return err - }) + plans, err := g.planAllWithinBudget(ctx, reg, client) if err != nil { return nil, err } @@ -98,8 +90,21 @@ func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Accoun return results, g.recordDeployments(ctx, client, results) } -// planAll decides what every instance binds, and verifies every recorded address. -// It deploys nothing and sends nothing. +func (g *generatorBuilder) planAllWithinBudget(ctx context.Context, + reg *registry.Registry, client *ethclient.Client, +) ([]plan, error) { + var plans []plan + err := loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", + func(ctx context.Context) error { + var err error + plans, err = g.planAll(ctx, reg, client) + return err + }) + return plans, err +} + +// planAll deploys nothing and sends nothing, which is what makes AS-3.3 hold for +// a profile of more than one scenario. func (g *generatorBuilder) planAll(ctx context.Context, reg *registry.Registry, client *ethclient.Client, ) ([]plan, error) { @@ -205,8 +210,6 @@ func (g *generatorBuilder) deployPlanned(ctx context.Context, plans []plan, return results, nil } -// bindAll marks every scenario ready and hands each contract scenario its bound -// instance, using the one client the step already holds. func (g *generatorBuilder) bindAll(client *ethclient.Client, results []prepared) error { for _, result := range results { if err := result.instance.Scenario.Ready(g.config); err != nil { @@ -234,8 +237,6 @@ func contractNameFor(instance *scenarioInstance) string { return strings.ToLower(instance.Config.Name) } -// describeHash renders a genesis hash for a log line, naming the empty case -// rather than printing nothing. func describeHash(hash string) string { if hash == "" { return "(empty)" @@ -243,9 +244,8 @@ func describeHash(hash string) string { return hash } -// logRegistrySource says which chain this run matched, and which file supplied -// it. Listing every chain the registry holds would not answer the question an -// operator has, which is whether their own chain was found. +// logRegistrySource reports the chain this run matched, not every chain the +// registry holds: the operator's question is whether their own chain was found. func logRegistrySource(reg *registry.Registry, chainID int64, genesisHash string) { chain, ok := reg.Chain(chainID, genesisHash) if !ok { @@ -258,9 +258,8 @@ func logRegistrySource(reg *registry.Registry, chainID int64, genesisHash string chain.ChainName, chainID, reg.Sources()[chain.ChainName], len(chain.Contracts)) } -// mockPrepareAll marks every scenario ready and binds each contract against a -// nil backend, which is the unit-test path. A nil backend builds transactions -// and never sends one. +// mockPrepareAll binds against a nil backend, which builds transactions and +// never sends one. func (g *generatorBuilder) mockPrepareAll() error { for _, instance := range g.instances { if err := instance.Scenario.Ready(g.config); err != nil { @@ -278,14 +277,12 @@ func (g *generatorBuilder) mockPrepareAll() error { // recordDeployments writes a chain file describing what this run deployed, for // an operator to review and commit. // -// It writes nothing when the run deployed nothing, and nothing when no path is -// configured. In a pod the only writable path is /dev/stdout: both the nightly -// Job and the canary Deployments mount their volumes read-only and set -// readOnlyRootFilesystem. -// // An ephemeral chain's file is not committed. That chain disappears after the // run, so an entry naming it could never verify again, and every later run // reading it would fail. +// +// In a pod the only writable path is /dev/stdout: both deployed shapes mount +// their volumes read-only and set readOnlyRootFilesystem. func (g *generatorBuilder) recordDeployments(ctx context.Context, client *ethclient.Client, results []prepared, ) error { diff --git a/main.go b/main.go index 35f02bc..c190372 100644 --- a/main.go +++ b/main.go @@ -137,6 +137,11 @@ func runLoadTest(ctx context.Context, cmd *cobra.Command) error { if recordPath, err := cmd.Flags().GetString("chain-record-path"); err == nil && recordPath != "" { cfg.ChainRecordPath = recordPath } + // Runs here rather than in loadConfig, because the flags above set the + // fields it reads. + if err := cfg.ValidateRecording(); err != nil { + return err + } log.Printf("🚀 Starting Sei Chain Load Test v2") log.Printf("📁 Config file: %s", configFile) diff --git a/registry/chains/README.md b/registry/chains/README.md index 882a409..d6eb950 100644 --- a/registry/chains/README.md +++ b/registry/chains/README.md @@ -10,15 +10,27 @@ fails every run that reads it. ## Adding a chain -A run against a chain with no entry deploys its contracts and writes a chain -file next to its run report. Review that file and commit it here. Do not write -one by hand: the code hash has to be the hash the chain actually serves, and a -run observes it rather than guessing. +Bootstrap a long-lived chain locally, not from a deployed pod. Run seiload +against the chain with `--chain-record-path ./deployed.json`. It deploys what the +registry does not name, reads each address back, and writes the file. Review it +and commit it here. -`WriteChain` refuses to write into this directory. Pointing a run at the -committed registry does not let it edit the committed registry. +Do not write one by hand: the code hash has to be the hash the chain actually +serves, and a run observes it rather than guessing. + +Do not commit a file from an ephemeral chain. That chain disappears after the +run, so the entry could never verify again and every later run reading it would +fail. + +`WriteChain` refuses a path under `chains/` or `registry/chains/`. That is a +guardrail against pointing a run at the committed registry, not a sandbox: a +symlink still reaches it. ## The format is frozen -Adding a field is fine. Renaming or repurposing one is a migration, because -every committed file has to change with it. See `registry/doc.go`. +Renaming or repurposing a field is forbidden — every committed file, and every +file a deployment supplies by path, has to change with it. + +Adding one is not free either. The registry rejects an unknown field, so a binary +older than the field fails to parse a file that carries it. Ship the reading +binary before the writing one. See `registry/doc.go`. diff --git a/registry/registry.go b/registry/registry.go index cfb7ed1..18cac82 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -101,7 +101,6 @@ func Load(paths ...string) (*Registry, error) { return r, nil } -// addEmbedded reads every chain file compiled into the binary. func (r *Registry) addEmbedded() error { entries, err := fs.ReadDir(embeddedChains, "chains") if err != nil { @@ -123,8 +122,7 @@ func (r *Registry) addEmbedded() error { return nil } -// add parses one chain file and records it under its identity, noting which -// source supplied it. A later source replaces an earlier one. +// add records a chain under its identity. A later source replaces an earlier one. func (r *Registry) add(data []byte, source string) error { var chain Chain if err := decodeStrict(data, &chain); err != nil { @@ -140,9 +138,9 @@ func (r *Registry) add(data []byte, source string) error { return nil } -// validate rejects a chain file missing a field the format requires. A file -// missing its identity would match nothing, and a contract missing its address -// would resolve to the zero address, which holds no code on any chain. +// validate rejects a file that would resolve to the wrong thing. A missing +// identity matches nothing; a missing address resolves to the zero address, +// which holds no code on any chain. func (c Chain) validate() error { if c.ChainID == 0 { return errors.New("chainId is required") @@ -210,11 +208,8 @@ func (r *Registry) Sources() map[string]string { return sources } -// decodeStrict unmarshals JSON into v. It rejects a key that maps to no field, -// and data after the value, which json.Unmarshal also rejects. -// -// This mirrors config.decodeStrict deliberately. A chain file that silently -// ignores a misspelled key would resolve a contract nobody configured. +// decodeStrict mirrors config.decodeStrict. A chain file that silently ignored a +// misspelled key would resolve a contract nobody configured. func decodeStrict(data []byte, v any) error { decoder := json.NewDecoder(bytes.NewReader(data)) decoder.DisallowUnknownFields() diff --git a/registry/resolve.go b/registry/resolve.go index db756de..f79be34 100644 --- a/registry/resolve.go +++ b/registry/resolve.go @@ -51,7 +51,9 @@ func (e *MismatchError) Error() string { } // Verify checks that the code at the recorded address hashes to the recorded -// hash. It returns a *MismatchError when it does not, and it never deploys. +// hash. It returns a *MismatchError when the code differs, a plain error when the +// chain names no such contract or the read fails, and nil otherwise. It never +// deploys. // // A run calls this before it sends any transaction. A call to an address holding // no code succeeds at the EVM layer and does nothing, and a call to an address @@ -82,10 +84,9 @@ func Verify(ctx context.Context, code CodeReader, chain Chain, name string) erro } } -// codeHashAt reads the runtime code at addr and hashes it the way the EVM -// defines an account's code hash. An address holding no code hashes to the zero -// hash rather than to Keccak-256 of the empty string, so a caller can tell -// "absent" from "present but different" by comparing against the zero value. +// codeHashAt returns the zero hash for an address holding no code, rather than +// Keccak-256 of the empty string. That is what lets a caller tell "absent" from +// "present but different" by comparing against the zero value. func codeHashAt(ctx context.Context, code CodeReader, addr common.Address) (common.Hash, error) { runtime, err := code.CodeAt(ctx, addr, nil) if err != nil { @@ -97,14 +98,10 @@ func codeHashAt(ctx context.Context, code CodeReader, addr common.Address) (comm return crypto.Keccak256Hash(runtime), nil } -// VerifyHasCode reports whether an address holds any contract code. It is the -// check for an address the registry does not describe, where there is no -// recorded hash to compare against. -// -// A call to an address holding no code succeeds at the EVM layer and does -// nothing, and a load generator that reads inclusion rather than execution -// cannot see that. This is the difference between an address nobody verified and -// a run that measures nothing. +// VerifyHasCode is the check for an address the registry does not describe, so +// there is no recorded hash to compare against. Without it a typo produces a full +// run of green metrics: the calls succeed at the EVM layer and do nothing, and +// sei-load reads inclusion rather than execution. func VerifyHasCode(ctx context.Context, code CodeReader, addr common.Address) error { hash, err := codeHashAt(ctx, code, addr) if err != nil { @@ -180,15 +177,13 @@ const ( var errWriteToEmbedded = errors.New( "refusing to write inside the compiled-in " + embeddedDirName + " directory") -// checkWritePath refuses a path that names a file inside the compiled-in chains -// directory. That refusal is what makes the read-only rule enforceable rather -// than a convention: a run pointed at the committed registry still cannot edit -// it. +// checkWritePath refuses a path whose directory is "chains" or +// "registry/chains", which are the two a run can realistically be given from +// inside the repo. // -// The binary does not know where its own source tree is, so this matches the two -// paths a run can realistically be given from inside the repo: "registry/chains" -// and a bare "chains". It is a guardrail against that mistake, not a sandbox — -// an absolute path to the same directory under another name still writes. +// It is a guardrail against that one mistake, not a sandbox. The binary does not +// know where its own source tree is, so a path reaching the same directory by +// another route — a symlink, a bind mount — still writes. func checkWritePath(p string) error { dir := path.Dir(path.Clean(filepath.ToSlash(p))) if dir == embeddedDirName || path.Base(dir) == embeddedDirName && From 60e11cbc04c927838bf19cce80aa2190755072ca Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 12:03:50 -0700 Subject: [PATCH 6/9] feat(registry): settle the three chain-file format decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chain file is a one-way door and these three sit inside it. Approved 2026-08-24 before any file is committed, which is the last moment they are free. Contract names are lowercase. The scenario factory already resolves a name that way, every profile in this repo writes CamelCase, and the constants are lowercase. Keying on the raw string would record "ERC20Conflict" from one profile and miss it from another that wrote "erc20conflict". A scenario may set contractKey to override the key its contract is recorded under. Two runs driving one chain must not share a contract: they write the same storage slots, and that contention is in neither profile. The three arctic-1 canary cells run one profile against one chain, so without distinct keys all three would measure a workload nobody configured. Long-lived chains are committed; re-genesised ones are supplied. pacific-1 and atlantic-2 keep their addresses, so committing puts them in a reviewed pull request and a signed image. arctic-1 changes its genesis hash on every re-genesis, and recovering a committed entry needs a sei-load pull request, a CI build, and a hand-edited image pin per cell — where the three cells are pinned independently on purpose. The cost of supplying a file is stated rather than left implicit: whoever can edit that source can name a contract of their choosing, and the code-hash check cannot catch it, because it proves the address holds the code the file recorded and not that the code is ours. The canary mounts a funded key, so the loss is bounded by that key's balance. That bound is why this is acceptable for arctic-1 and not for pacific-1. Verifiers: gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing. Co-Authored-By: Claude Opus 5 (1M context) --- config/config.go | 9 +++++++++ generator/prepare.go | 15 +++++++++------ generator/registry_test.go | 39 ++++++++++++++++++++++++++++++++++++++ registry/chains/README.md | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 8d38adc..c7a716e 100644 --- a/config/config.go +++ b/config/config.go @@ -186,6 +186,15 @@ type Scenario struct { // by operation name. Absent (the default) selects the scenario's first // declared operation; see operation.go. Operations OperationMix `json:"operations,omitempty"` + // ContractKey is the name this scenario's contract is recorded under in the + // chain file. It defaults to Name. + // + // Set it where two runs drive one chain and must not share a contract. The + // three arctic-1 canary cells are the case: they run one profile against one + // chain, so without distinct keys they bind one contract and contend on its + // storage. That contention is in neither profile, so all three would measure + // a workload nobody configured. + ContractKey string `json:"contractKey,omitempty"` // ContractAddress names a contract deployed outside this repo. Set, the run // binds it and consults no registry and deploys nothing. It is the escape // hatch for a contract the registry does not and should not describe. diff --git a/generator/prepare.go b/generator/prepare.go index 4907df7..2ffe2ea 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -226,14 +226,17 @@ func (g *generatorBuilder) bindAll(client *ethclient.Client, results []prepared) return nil } -// contractNameFor is the key a scenario's contract is recorded under. +// contractNameFor is the key a scenario's contract is recorded under: its +// contractKey, or its scenario name. // -// It lowercases the profile's configured name, matching how the scenario factory -// resolves that same name. Every profile in this repo writes CamelCase, and the -// scenario constants are lowercase, so keying on the raw string would record -// "ERC20Conflict" from one profile and miss it from another that wrote -// "erc20conflict". One vocabulary, one case. +// It lowercases either, matching how the scenario factory resolves a name. Every +// profile in this repo writes CamelCase and the scenario constants are +// lowercase, so keying on the raw string would record "ERC20Conflict" from one +// profile and miss it from another that wrote "erc20conflict". func contractNameFor(instance *scenarioInstance) string { + if key := instance.Config.ContractKey; key != "" { + return strings.ToLower(key) + } return strings.ToLower(instance.Config.Name) } diff --git a/generator/registry_test.go b/generator/registry_test.go index d26019e..d914a4d 100644 --- a/generator/registry_test.go +++ b/generator/registry_test.go @@ -436,3 +436,42 @@ func TestContractNameIsCaseInsensitive(t *testing.T) { require.Equal(t, recordedAddress, *tx.EthTx.To()) } } + +// TestContractKeySeparatesTwoRunsOnOneChain asserts the per-cell identity rule. +// Two profiles driving the same scenario against the same chain get their own +// contracts when they set distinct contractKeys, which is what keeps each cell's +// contention its own. +func TestContractKeySeparatesTwoRunsOnOneChain(t *testing.T) { + first := common.HexToAddress("0x0000000000000000000000000000000000000A01") + second := common.HexToAddress("0x0000000000000000000000000000000000000A02") + chain := newMockChain(t, mockChainConfig{ + code: map[common.Address][]byte{first: recordedCode, second: recordedCode}, + }) + + // One chain file, two entries, one per cell. + path := filepath.Join(t.TempDir(), "chain.json") + require.NoError(t, registry.WriteChain(path, registry.Chain{ + ChainID: testChainID, ChainName: testChainName, GenesisHash: testGenesisHash, + Contracts: []registry.Contract{ + {Name: "storagerw-euw1", Address: first, CodeHash: crypto.Keccak256Hash(recordedCode)}, + {Name: "storagerw-use2", Address: second, CodeHash: crypto.Keccak256Hash(recordedCode)}, + }, + })) + + bound := func(t *testing.T, key string) common.Address { + t.Helper() + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.Scenarios[0].ContractKey = key + cfg.ChainFiles = []string{path} + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.NoError(t, err) + txs := generateN(t, newTestRng(1), gen, 3) + require.NotEmpty(t, txs) + return *txs[0].EthTx.To() + } + + require.Equal(t, first, bound(t, "storagerw-euw1")) + require.Equal(t, second, bound(t, "storagerw-use2")) + require.Zero(t, chain.txCount(), "a keyed lookup deployed instead of binding") +} diff --git a/registry/chains/README.md b/registry/chains/README.md index d6eb950..851fbe9 100644 --- a/registry/chains/README.md +++ b/registry/chains/README.md @@ -26,6 +26,39 @@ fail. guardrail against pointing a run at the committed registry, not a sandbox: a symlink still reaches it. +## Which chains belong here + +Split by how long the chain lives, not by convenience. + +**Commit here: pacific-1 and atlantic-2.** They are never re-genesised, so a +recorded address stays true. Committing puts the address in a reviewed pull +request and in a signed image, next to the bindings that encode against it. + +**Supply by `--chain-file` instead: arctic-1, and every devnet.** arctic-1 is +re-genesised on a devnet cadence, and each re-genesis changes its genesis hash +and invalidates a committed entry. Recovering from that here means a sei-load +pull request, a CI build, and a hand-edited image pin in each cell — where the +three canary cells are pinned independently on purpose. A file the deployment +supplies recovers with one reconcile. + +The cost of supplying it is real and worth stating: anyone who can edit that +source can point a run at a contract of their choosing, and the code-hash check +cannot catch it — the check proves the address holds the code the *file* +recorded, not that the code is ours. The canary mounts a funded key, so the loss +is bounded by that key's balance. That bound is the reason this is acceptable for +arctic-1 and not for pacific-1. + +**Never commit an ephemeral chain's file.** It disappears after the run, so the +entry could never verify again. + +## Two runs on one chain + +Two runs that drive one chain must not share a contract: they would write the +same storage slots, and that contention is in neither profile. + +Give each one its own `contractKey` in its profile, and one entry per key in the +chain file. The three arctic-1 canary cells are the case this exists for. + ## The format is frozen Renaming or repurposing a field is forbidden — every committed file, and every From ff25edd035a01d7ddd349c252b9a14cbe14f6280 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 12:34:11 -0700 Subject: [PATCH 7/9] fix(generator): one contract per name, and rename prepared to binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot caught a defect my own duplicate-name check introduced. A profile naming one scenario twice deployed two contracts, then WriteChain rejected the file for holding two entries under one name — so the run died after both deployments, leaving them on-chain and unrecorded. That is the same failure class as the AS-3.3 violation this branch already fixed, reintroduced by the fix for a different finding. Reproduced before fixing. The root cause was two vocabularies. createScenarios suffixes the display name to storagerw_0 and storagerw_1, while contractNameFor keys on the unsuffixed profile name, so two instances shared one key. Resolution now groups instances by contract name: resolve once, deploy once, bind every instance in the group. That also closes the asymmetry the systems review raised separately — deploying per instance gave a fresh chain two contracts and a covered chain one, from the same profile, so the same profile measured a different workload depending on which chain it ran against. An operator who wants two contracts sets a distinct contractKey on each, which is what that field is for. Renames prepared to binding, per review. A past participle reads as something that happened rather than a thing the code holds; binding names what it is, and pairs with the plan-then-bind sequence around it. prepareAll now reads as five named steps: plan, deploy what is missing, ready, bind, record. Verifiers: gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing. Co-Authored-By: Claude Opus 5 (1M context) --- generator/prepare.go | 196 +++++++++++++++++++++---------------- generator/registry_test.go | 43 ++++++++ 2 files changed, 152 insertions(+), 87 deletions(-) diff --git a/generator/prepare.go b/generator/prepare.go index 2ffe2ea..a6701b3 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -23,22 +23,23 @@ import ( // would hold startup open with nothing logged. const resolveTimeout = 60 * time.Second -// plan is what one scenario instance will bind, decided before anything deploys. -type plan struct { - instance *scenarioInstance - // address is the address to bind, or zero when this instance must deploy. +// binding is one contract the profile drives: the instances that share it, the +// address they bind, and whether this run created it. +// +// Instances are grouped by contract name rather than listed one per scenario. A +// profile may name one scenario twice, and both entries drive the same contract +// unless each sets its own contractKey — so the run resolves once, deploys once, +// and binds both. Deploying per instance instead would give a covered chain one +// contract and a fresh chain two, from the same profile. +type binding struct { + name string + instances []*scenarioInstance + // address is the zero address until deployMissing fills it in. A contract + // nothing named has no address, and no chain holds code at the zero address, + // so the zero value is the signal to deploy. address common.Address - // deploy is true when the registry named nothing and the run must create it. - deploy bool -} - -// prepared reports what one scenario instance ended up bound to, so the caller -// can record a fresh deployment. -type prepared struct { - instance *scenarioInstance - address common.Address - // deployed is true when this run created the contract, which is what makes - // it worth recording. A resolved address is already recorded. + // deployed is true when this run created the contract, which is what makes it + // worth recording. A resolved address is already recorded. deployed bool } @@ -55,7 +56,7 @@ type prepared struct { // code, and it binds. It is not the client that sends load: CreateTransactionOpts // sets auth.NoSend, so a bound contract hands the transaction back rather than // sending it, and the sender's own per-endpoint clients send. -func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) ([]prepared, error) { +func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) ([]*binding, error) { if g.config.MockDeploy { return nil, g.mockPrepareAll() } @@ -75,61 +76,77 @@ func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Accoun } defer client.Close() - plans, err := g.planAllWithinBudget(ctx, reg, client) + bindings, err := g.planAllWithinBudget(ctx, reg, client) if err != nil { return nil, err } - - results, err := g.deployPlanned(ctx, plans, deployer) - if err != nil { + if err := g.deployMissing(ctx, bindings, deployer); err != nil { + return nil, err + } + if err := g.readyAll(); err != nil { return nil, err } - if err := g.bindAll(client, results); err != nil { + if err := g.bindAll(client, bindings); err != nil { return nil, err } - return results, g.recordDeployments(ctx, client, results) + return bindings, g.recordDeployments(ctx, client, bindings) } func (g *generatorBuilder) planAllWithinBudget(ctx context.Context, reg *registry.Registry, client *ethclient.Client, -) ([]plan, error) { - var plans []plan +) ([]*binding, error) { + var bindings []*binding err := loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", func(ctx context.Context) error { var err error - plans, err = g.planAll(ctx, reg, client) + bindings, err = g.planAll(ctx, reg, client) return err }) - return plans, err + return bindings, err } -// planAll deploys nothing and sends nothing, which is what makes AS-3.3 hold for -// a profile of more than one scenario. +// planAll groups the contract scenarios by the name their contract is recorded +// under, then decides one address per group. It deploys nothing and sends +// nothing, which is what makes AS-3.3 hold for a profile of more than one. func (g *generatorBuilder) planAll(ctx context.Context, reg *registry.Registry, client *ethclient.Client, -) ([]plan, error) { - plans := make([]plan, 0, len(g.instances)) +) ([]*binding, error) { + var bindings []*binding + byName := make(map[string]*binding) + for _, instance := range g.instances { if instance.Scenario.Binder() == nil { - plans = append(plans, plan{instance: instance}) continue } - p, err := g.planOne(ctx, reg, client, instance) - if err != nil { - return nil, fmt.Errorf("prepare %s: %w", instance.Name, err) + name := contractNameFor(instance) + if existing, ok := byName[name]; ok { + existing.instances = append(existing.instances, instance) + continue + } + b := &binding{name: name, instances: []*scenarioInstance{instance}} + byName[name] = b + bindings = append(bindings, b) + } + + for _, b := range bindings { + if err := g.planOne(ctx, reg, client, b); err != nil { + return nil, fmt.Errorf("prepare %s: %w", b.name, err) } - plans = append(plans, p) } - return plans, nil + return bindings, nil } -// planOne decides which address one contract scenario binds, in a fixed order of +// planOne decides which address one contract binds, in a fixed order of // precedence: an explicit address from the profile, a forced deployment, a // registry entry, then a deployment because nothing named one. +// +// It reads the config of the group's first instance. Two instances sharing a +// contract name share its address, so they must agree on how that address is +// chosen; validate rejects the config where they do not. func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, - client *ethclient.Client, instance *scenarioInstance, -) (plan, error) { - cfg := instance.Config + client *ethclient.Client, b *binding, +) error { + cfg := b.instances[0].Config if cfg.ContractAddress != "" { address := common.HexToAddress(cfg.ContractAddress) @@ -138,27 +155,28 @@ func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, // against an address that holds nothing: the calls succeed at the EVM // layer, do nothing, and sei-load reads inclusion rather than execution. if err := registry.VerifyHasCode(ctx, client, address); err != nil { - return plan{}, fmt.Errorf("contractAddress %s: %w", address, err) + return fmt.Errorf("contractAddress %s: %w", address, err) } - log.Printf("📌 %s: binding the configured address %s", instance.Name, address) - return plan{instance: instance, address: address}, nil + log.Printf("📌 %s: binding the configured address %s", b.name, address) + b.address = address + return nil } if cfg.ForceDeploy { - log.Printf("🔁 %s: forceDeploy is set, deploying rather than resolving", instance.Name) - return plan{instance: instance, deploy: true}, nil + log.Printf("🔁 %s: forceDeploy is set, deploying rather than resolving", b.name) + return nil } - name := contractNameFor(instance) address, mustDeploy, err := registry.Resolve(ctx, client, reg, - g.config.ChainID, g.config.GenesisHash, name) + g.config.ChainID, g.config.GenesisHash, b.name) if err != nil { - return plan{}, err + return err } if !mustDeploy { log.Printf("📖 %s: bound the recorded address %s, no deployment sent", - instance.Name, address) - return plan{instance: instance, address: address}, nil + b.name, address) + b.address = address + return nil } // A miss because the run named no genesis hash is a misconfiguration, not a @@ -166,61 +184,66 @@ func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, // and would deploy on every restart in silence — the behaviour this feature // exists to remove. Fail rather than degenerate. if g.config.GenesisHash == "" && reg.HasChainID(g.config.ChainID) { - return plan{}, fmt.Errorf( + return fmt.Errorf( "the registry describes chain %d but this run names no genesisHash, "+ - "so it matches nothing and would deploy %s again on every restart. "+ + "so it matches nothing and would deploy again on every restart. "+ "Set genesisHash in the profile, or set forceDeploy on this scenario", - g.config.ChainID, name) + g.config.ChainID) } - log.Printf("🆕 %s: no registry entry for %s on chain %d (genesisHash %s), deploying", - instance.Name, name, g.config.ChainID, describeHash(g.config.GenesisHash)) - return plan{instance: instance, deploy: true}, nil + log.Printf("🆕 %s: no registry entry on chain %d (genesisHash %s), deploying", + b.name, g.config.ChainID, describeHash(g.config.GenesisHash)) + return nil } -// deployPlanned creates every contract the plan says is missing. Every recorded -// address has already verified by the time this runs. +// deployMissing creates every contract the plan left without an address. Every +// recorded address has already verified by the time this runs. // // It stays sequential: each deployment reads its nonce from the chain and is // mined before the next is sent, so one deployer key stays in one ordered nonce // stream. Deploying concurrently reintroduces the nonce collision the sequence // prevents. -func (g *generatorBuilder) deployPlanned(ctx context.Context, plans []plan, +func (g *generatorBuilder) deployMissing(ctx context.Context, bindings []*binding, deployer types.Account, -) ([]prepared, error) { - results := make([]prepared, 0, len(plans)) - for _, p := range plans { - if !p.deploy { - results = append(results, prepared{instance: p.instance, address: p.address}) +) error { + for _, b := range bindings { + if b.address != (common.Address{}) { continue } if deployer.PrivKey == nil { - return nil, errors.New( + return errors.New( "deployer has no private key (a live deployment must be signed)") } - log.Printf("Deploying scenario %s", p.instance.Name) - address, err := p.instance.Scenario.Deploy(ctx, g.config, deployer) + log.Printf("Deploying %s", b.name) + address, err := b.instances[0].Scenario.Deploy(ctx, g.config, deployer) if err != nil { - return nil, fmt.Errorf("prepare %s: %w", p.instance.Name, err) + return fmt.Errorf("prepare %s: %w", b.name, err) } - log.Printf("🚀 Deployed %s at address: %s", p.instance.Name, address) - results = append(results, prepared{ - instance: p.instance, address: address, deployed: true, - }) + log.Printf("🚀 Deployed %s at address: %s", b.name, address) + b.address = address + b.deployed = true } - return results, nil + return nil } -func (g *generatorBuilder) bindAll(client *ethclient.Client, results []prepared) error { - for _, result := range results { - if err := result.instance.Scenario.Ready(g.config); err != nil { +// readyAll marks every scenario able to generate, including the ones that drive +// no contract. +func (g *generatorBuilder) readyAll() error { + for _, instance := range g.instances { + if err := instance.Scenario.Ready(g.config); err != nil { return err } - binder := result.instance.Scenario.Binder() - if binder == nil { - continue - } - if err := binder(client, result.address); err != nil { - return fmt.Errorf("prepare %s: %w", result.instance.Name, err) + } + return nil +} + +// bindAll hands every instance its bound contract, using the one client the step +// already holds. +func (g *generatorBuilder) bindAll(client *ethclient.Client, bindings []*binding) error { + for _, b := range bindings { + for _, instance := range b.instances { + if err := instance.Scenario.Binder()(client, b.address); err != nil { + return fmt.Errorf("prepare %s: %w", instance.Name, err) + } } } return nil @@ -287,7 +310,7 @@ func (g *generatorBuilder) mockPrepareAll() error { // In a pod the only writable path is /dev/stdout: both deployed shapes mount // their volumes read-only and set readOnlyRootFilesystem. func (g *generatorBuilder) recordDeployments(ctx context.Context, - client *ethclient.Client, results []prepared, + client *ethclient.Client, bindings []*binding, ) error { if g.config.ChainRecordPath == "" { return nil @@ -304,12 +327,11 @@ func (g *generatorBuilder) recordDeployments(ctx context.Context, err := loadutils.WithinBudget(ctx, resolveTimeout, "recording deployments", func(ctx context.Context) error { - for _, result := range results { - if !result.deployed { + for _, b := range bindings { + if !b.deployed { continue } - contract, err := registry.Record(ctx, client, - contractNameFor(result.instance), result.address) + contract, err := registry.Record(ctx, client, b.name, b.address) if err != nil { return err } diff --git a/generator/registry_test.go b/generator/registry_test.go index d914a4d..3551b99 100644 --- a/generator/registry_test.go +++ b/generator/registry_test.go @@ -475,3 +475,46 @@ func TestContractKeySeparatesTwoRunsOnOneChain(t *testing.T) { require.Equal(t, second, bound(t, "storagerw-use2")) require.Zero(t, chain.txCount(), "a keyed lookup deployed instead of binding") } + +// TestDuplicateScenarioNamesShareOneContract asserts that a profile naming one +// scenario twice deploys once and binds both instances to it. +// +// The two paths must agree. Deploying per instance would give a fresh chain two +// contracts and a covered chain one, from the same profile — so the same profile +// would measure a different workload depending on which chain it ran against. +// An operator who wants two contracts sets a distinct contractKey on each. +func TestDuplicateScenarioNamesShareOneContract(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + recordPath := filepath.Join(t.TempDir(), "out.json") + cfg := &config.LoadConfig{ + ChainID: testChainID, GenesisHash: testGenesisHash, + SeiChainID: testChainName, + Endpoints: []string{chain.url}, + Accounts: &config.AccountConfig{Accounts: 4}, + Scenarios: []config.Scenario{ + {Name: scenarios.StorageRW, Weight: 1}, + {Name: scenarios.StorageRW, Weight: 1}, + }, + ChainRecordPath: recordPath, + } + + deployer := types.NewAccount(false) + gen, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, deployer) + require.NoError(t, err, "the run deployed and then failed to record") + + require.Equal(t, 1, chain.txCount(), + "two instances of one scenario deployed %d contracts", chain.txCount()) + + deployedAt := crypto.CreateAddress(deployer.Address, 0) + for _, tx := range generateN(t, newTestRng(1), gen, 8) { + require.Equal(t, deployedAt, *tx.EthTx.To(), + "an instance bound something other than the one deployed contract") + } + + // And the record it wrote is a file Load accepts, with one entry. + reloaded, err := registry.Load(recordPath) + require.NoError(t, err) + written, ok := reloaded.Chain(testChainID, testGenesisHash) + require.True(t, ok) + require.Len(t, written.Contracts, 1) +} From c1bca6a136c44e461201d81c827107b52f8b77b4 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 12:42:04 -0700 Subject: [PATCH 8/9] fix(generator): guard the group's address selection, and move the recording check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of the systems review verified F1, F2, F4, F5, F6 and F15 closed against real probes. It found one new defect and one incomplete fix, both reproduced before this change. Grouping instances by contract name let the group silently inherit the first instance's selection. forceDeploy on a second instance was dropped without a word, and config validation accepted it: an operator sets forceDeploy to get virgin storage for a contention measurement and binds a contract carrying another run's state instead. That is the hazard the comment on ContractAddress already names, moved from the field to the group. groupByContractName now rejects a group whose instances disagree, and names contractKey as the way to have two contracts on purpose. planOne's doc had asserted "validate rejects the config where they do not". Nothing did. A comment long enough to assert a clause, plausible enough that nobody checked — the P13 failure mode, inside the change that cites P13. The clause is now true because the guard exists and the doc names it. ValidateRecording sat only in main, so NewGenerator still deployed a contract and then failed the write the function exists to pre-empt. It now runs at prepareAll as well, which is the function every caller of the package reaches. main keeps its call for the early exit before the metrics server starts. MismatchError asserted a cause it had not checked. A lagging or syncing endpoint produces the identical symptom, and the run reads one endpoint at latest. It now states what was observed and offers both causes, so nobody chases a re-genesis that did not happen at 3am. Structure, from the same review: the budget wrapper is folded into planAll and now covers only the chain reads, which is what its constant says it bounds; grouping needs no timeout. prepareAll returns an error rather than a slice nobody read. recordDeployments gets its own line instead of riding a return that paired a non-nil value with a non-nil error. The doc said "three passes" in the commit that made it five. Two comments moved rather than being deleted. The read-only-pod claim was a cross-repo invariant a comment cannot hold; it is now in the --chain-record-path flag help, where the operator who needs it reads it. The duplicated green-metrics rationale now lives once, on VerifyHasCode. Verifiers: git status clean, gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing. Reporting the tree state explicitly because last round I called it clean while an untracked failing probe sat in it. Co-Authored-By: Claude Opus 5 (1M context) --- config/config.go | 13 ++-- generator/generator.go | 2 +- generator/prepare.go | 124 ++++++++++++++++++++++--------------- generator/registry_test.go | 81 ++++++++++++++++++++++++ main.go | 2 +- registry/registry.go | 4 +- registry/resolve.go | 4 +- 7 files changed, 166 insertions(+), 64 deletions(-) diff --git a/config/config.go b/config/config.go index c7a716e..cd8ba15 100644 --- a/config/config.go +++ b/config/config.go @@ -189,11 +189,9 @@ type Scenario struct { // ContractKey is the name this scenario's contract is recorded under in the // chain file. It defaults to Name. // - // Set it where two runs drive one chain and must not share a contract. The - // three arctic-1 canary cells are the case: they run one profile against one - // chain, so without distinct keys they bind one contract and contend on its - // storage. That contention is in neither profile, so all three would measure - // a workload nobody configured. + // Set it where two runs drive one chain and must not share a contract. + // Without distinct keys they bind one contract and contend on its storage, + // and that contention is in neither profile. ContractKey string `json:"contractKey,omitempty"` // ContractAddress names a contract deployed outside this repo. Set, the run // binds it and consults no registry and deploys nothing. It is the escape @@ -283,8 +281,9 @@ func (s *Scenario) Validate() error { // contracts on the chain, records none of them, and exits non-zero. Refusing up // front costs nothing and leaves the chain untouched. // -// A caller MUST run this after merging command-line flags, because -// --chain-record-path sets the field this reads. +// main calls it after merging the flags, for an early exit before the metrics +// server starts. prepareAll calls it too, so no caller of this package can skip +// it. func (c *LoadConfig) ValidateRecording() error { if c.ChainRecordPath == "" || c.GenesisHash != "" { return nil diff --git a/generator/generator.go b/generator/generator.go index bd3ed52..357e9bc 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -236,7 +236,7 @@ func NewGenerator(ctx context.Context, rng *mrand.Rand, cfg *config.LoadConfig, } // Step 2: give every scenario the contract it drives - if _, err := b.prepareAll(ctx, deployer); err != nil { + if err := b.prepareAll(ctx, deployer); err != nil { return nil, fmt.Errorf("failed to prepare scenarios: %w", err) } diff --git a/generator/prepare.go b/generator/prepare.go index a6701b3..0725cc3 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -43,106 +43,133 @@ type binding struct { deployed bool } -// prepareAll gives every scenario instance the contract it drives, and reports -// what it deployed. +// prepareAll gives every scenario instance the contract it drives. // -// It runs in three passes, and the order is the point. Every address is decided +// The order is the point. Every address is decided // and every recorded address verified before anything deploys, so a stale entry // on the last scenario cannot leave a contract from the first one on the chain, // paid for and recorded nowhere. AS-3.3 requires that a failed run deployed // nothing, and only the ordering makes it true for a profile of more than one. // -// One client serves all three passes and the recording that follows. It reads +// One client serves every step here. It reads // code, and it binds. It is not the client that sends load: CreateTransactionOpts // sets auth.NoSend, so a bound contract hands the transaction back rather than // sending it, and the sender's own per-endpoint clients send. -func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) ([]*binding, error) { +func (g *generatorBuilder) prepareAll(ctx context.Context, deployer types.Account) error { if g.config.MockDeploy { - return nil, g.mockPrepareAll() + return g.mockPrepareAll() } if len(g.config.Endpoints) == 0 { - return nil, errors.New("no endpoints configured, so no contract can be resolved") + return errors.New("no endpoints configured, so no contract can be resolved") + } + // Here as well as in main: this is the function every caller of the package + // reaches, and the check exists to fire before the first deployment. + if err := g.config.ValidateRecording(); err != nil { + return err } reg, err := registry.Load(g.config.ChainFiles...) if err != nil { - return nil, fmt.Errorf("load the contract registry: %w", err) + return fmt.Errorf("load the contract registry: %w", err) } logRegistrySource(reg, g.config.ChainID, g.config.GenesisHash) client, err := ethclient.Dial(g.config.Endpoints[0]) if err != nil { - return nil, fmt.Errorf("dial %s: %w", g.config.Endpoints[0], err) + return fmt.Errorf("dial %s: %w", g.config.Endpoints[0], err) } defer client.Close() - bindings, err := g.planAllWithinBudget(ctx, reg, client) + bindings, err := g.planAll(ctx, reg, client) if err != nil { - return nil, err + return err } if err := g.deployMissing(ctx, bindings, deployer); err != nil { - return nil, err + return err } if err := g.readyAll(); err != nil { - return nil, err + return err } if err := g.bindAll(client, bindings); err != nil { - return nil, err + return err } - return bindings, g.recordDeployments(ctx, client, bindings) + return g.recordDeployments(ctx, client, bindings) } -func (g *generatorBuilder) planAllWithinBudget(ctx context.Context, - reg *registry.Registry, client *ethclient.Client, +// planAll decides one address per contract the profile drives. It deploys +// nothing and sends nothing, which is what makes AS-3.3 hold for a profile of +// more than one. +func (g *generatorBuilder) planAll(ctx context.Context, reg *registry.Registry, + client *ethclient.Client, ) ([]*binding, error) { - var bindings []*binding - err := loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", + bindings, err := groupByContractName(g.instances) + if err != nil { + return nil, err + } + // The budget covers the chain reads and nothing else. Grouping touches no + // network. + return bindings, loadutils.WithinBudget(ctx, resolveTimeout, "contract resolution", func(ctx context.Context) error { - var err error - bindings, err = g.planAll(ctx, reg, client) - return err + for _, b := range bindings { + if err := g.planOne(ctx, reg, client, b); err != nil { + return fmt.Errorf("prepare %s: %w", b.name, err) + } + } + return nil }) - return bindings, err } -// planAll groups the contract scenarios by the name their contract is recorded -// under, then decides one address per group. It deploys nothing and sends -// nothing, which is what makes AS-3.3 hold for a profile of more than one. -func (g *generatorBuilder) planAll(ctx context.Context, reg *registry.Registry, - client *ethclient.Client, -) ([]*binding, error) { +// groupByContractName groups the contract scenarios by the name their contract +// is recorded under. Instances in one group share one address, so they must +// agree on how that address is chosen. +func groupByContractName(instances []*scenarioInstance) ([]*binding, error) { var bindings []*binding byName := make(map[string]*binding) - for _, instance := range g.instances { + for _, instance := range instances { if instance.Scenario.Binder() == nil { continue } name := contractNameFor(instance) - if existing, ok := byName[name]; ok { - existing.instances = append(existing.instances, instance) + existing, ok := byName[name] + if !ok { + b := &binding{name: name, instances: []*scenarioInstance{instance}} + byName[name] = b + bindings = append(bindings, b) continue } - b := &binding{name: name, instances: []*scenarioInstance{instance}} - byName[name] = b - bindings = append(bindings, b) - } - - for _, b := range bindings { - if err := g.planOne(ctx, reg, client, b); err != nil { - return nil, fmt.Errorf("prepare %s: %w", b.name, err) + if err := sameSelection(existing.instances[0], instance); err != nil { + return nil, err } + existing.instances = append(existing.instances, instance) } return bindings, nil } +// sameSelection rejects two instances of one contract that disagree on how its +// address is chosen. The group resolves once, so one selection would be dropped +// in silence, and the scenario that lost would measure against a contract nobody +// configured for it. +func sameSelection(first, next *scenarioInstance) error { + a, b := first.Config, next.Config + if a.ContractAddress == b.ContractAddress && a.ForceDeploy == b.ForceDeploy { + return nil + } + return fmt.Errorf( + "%s and %s both drive the contract named %q but choose its address "+ + "differently (contractAddress %q/%q, forceDeploy %v/%v). Give them "+ + "distinct contractKeys, or make the two agree", + first.Name, next.Name, contractNameFor(first), + a.ContractAddress, b.ContractAddress, a.ForceDeploy, b.ForceDeploy) +} + // planOne decides which address one contract binds, in a fixed order of // precedence: an explicit address from the profile, a forced deployment, a // registry entry, then a deployment because nothing named one. // -// It reads the config of the group's first instance. Two instances sharing a -// contract name share its address, so they must agree on how that address is -// chosen; validate rejects the config where they do not. +// It reads the config of the group's first instance. groupByContractName has +// already rejected a group whose instances disagree on how the address is +// chosen, so any instance would answer the same. func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, client *ethclient.Client, b *binding, ) error { @@ -150,10 +177,6 @@ func (g *generatorBuilder) planOne(ctx context.Context, reg *registry.Registry, if cfg.ContractAddress != "" { address := common.HexToAddress(cfg.ContractAddress) - // An explicit address skips the registry, and it must not skip the code - // check. A typo would otherwise produce a full run of green metrics - // against an address that holds nothing: the calls succeed at the EVM - // layer, do nothing, and sei-load reads inclusion rather than execution. if err := registry.VerifyHasCode(ctx, client, address); err != nil { return fmt.Errorf("contractAddress %s: %w", address, err) } @@ -226,7 +249,8 @@ func (g *generatorBuilder) deployMissing(ctx context.Context, bindings []*bindin } // readyAll marks every scenario able to generate, including the ones that drive -// no contract. +// no contract. Those are the instances groupByContractName skipped, so this is +// the only step that reaches them. func (g *generatorBuilder) readyAll() error { for _, instance := range g.instances { if err := instance.Scenario.Ready(g.config); err != nil { @@ -306,9 +330,7 @@ func (g *generatorBuilder) mockPrepareAll() error { // An ephemeral chain's file is not committed. That chain disappears after the // run, so an entry naming it could never verify again, and every later run // reading it would fail. -// -// In a pod the only writable path is /dev/stdout: both deployed shapes mount -// their volumes read-only and set readOnlyRootFilesystem. + func (g *generatorBuilder) recordDeployments(ctx context.Context, client *ethclient.Client, bindings []*binding, ) error { diff --git a/generator/registry_test.go b/generator/registry_test.go index 3551b99..c1ad3d3 100644 --- a/generator/registry_test.go +++ b/generator/registry_test.go @@ -518,3 +518,84 @@ func TestDuplicateScenarioNamesShareOneContract(t *testing.T) { require.True(t, ok) require.Len(t, written.Contracts, 1) } + +// TestGroupedInstancesMustAgreeOnSelection asserts the guard on the group. +// +// Two instances sharing a contract name share its address, so only one +// selection can win. Without the guard the loser is dropped in silence: an +// operator sets forceDeploy for virgin storage and gets a contract carrying +// another run's state, with the profile asking for the right thing. +func TestGroupedInstancesMustAgreeOnSelection(t *testing.T) { + elsewhere := "0x00000000000000000000000000000000000000EE" + + cases := []struct { + name string + second config.Scenario + }{ + {"forceDeploy on only one", config.Scenario{ + Name: scenarios.StorageRW, Weight: 1, ForceDeploy: true}}, + {"contractAddress on only one", config.Scenario{ + Name: scenarios.StorageRW, Weight: 1, ContractAddress: elsewhere}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + cfg := &config.LoadConfig{ + ChainID: testChainID, GenesisHash: testGenesisHash, + Endpoints: []string{chain.url}, + Accounts: &config.AccountConfig{Accounts: 4}, + Scenarios: []config.Scenario{ + {Name: scenarios.StorageRW, Weight: 1}, + tc.second, + }, + } + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err, "one instance's selection was dropped in silence") + require.ErrorContains(t, err, "distinct contractKeys") + require.Zero(t, chain.txCount(), + "the run deployed before rejecting the config") + }) + } + + // Two instances that agree still share one contract, which is the case + // TestDuplicateScenarioNamesShareOneContract covers. The guard rejects + // disagreement, not duplication. + t.Run("agreement is allowed", func(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + cfg := &config.LoadConfig{ + ChainID: testChainID, GenesisHash: testGenesisHash, + Endpoints: []string{chain.url}, + Accounts: &config.AccountConfig{Accounts: 4}, + Scenarios: []config.Scenario{ + {Name: scenarios.StorageRW, Weight: 1, ForceDeploy: true}, + {Name: scenarios.StorageRW, Weight: 1, ForceDeploy: true}, + }, + } + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.NoError(t, err) + require.Equal(t, 1, chain.txCount()) + }) +} + +// TestRecordingIsRefusedBeforeAnyDeployment asserts the recording precondition +// fires at the choke point, not only in main. +// +// A run configured to record but naming no genesisHash used to deploy every +// contract and then fail on the write, leaving them on-chain and unrecorded. +func TestRecordingIsRefusedBeforeAnyDeployment(t *testing.T) { + chain := newMockChain(t, mockChainConfig{}) + cfg := oneContractConfig(chain, scenarios.StorageRW) + cfg.GenesisHash = "" + cfg.ChainRecordPath = filepath.Join(t.TempDir(), "out.json") + + _, err := generator.NewGenerator(t.Context(), newTestRng(1), cfg, + types.NewAccount(false)) + require.Error(t, err) + require.ErrorContains(t, err, "genesisHash") + require.Zero(t, chain.txCount(), + "the run deployed %d contract(s) before refusing to record", chain.txCount()) +} diff --git a/main.go b/main.go index c190372..f22beb0 100644 --- a/main.go +++ b/main.go @@ -68,7 +68,7 @@ func init() { rootCmd.Flags().Bool("ramp-up", false, "Ramp up loadtest") rootCmd.Flags().String("report-path", "", "Path to save the report") rootCmd.Flags().StringArray("chain-file", nil, "Contract registry file describing the target chain and its deployed contracts. Repeatable; each layers over the registry compiled into the binary, and a later file wins.") - rootCmd.Flags().String("chain-record-path", "", "Where to write a chain file describing what this run deployed, for an operator to review and commit") + rootCmd.Flags().String("chain-record-path", "", "Where to write a chain file describing what this run deployed, for an operator to review and commit. In a pod use /dev/stdout: the deployed Job and canary Deployments mount their volumes read-only and set readOnlyRootFilesystem") rootCmd.Flags().String("txs-dir", "", "Path to save the transactions") rootCmd.Flags().Uint64("target-gas", 10_000_000, "Target gas per block") rootCmd.Flags().Int("num-blocks-to-write", 100, "Number of blocks to write") diff --git a/registry/registry.go b/registry/registry.go index 18cac82..8d45a69 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -208,8 +208,8 @@ func (r *Registry) Sources() map[string]string { return sources } -// decodeStrict mirrors config.decodeStrict. A chain file that silently ignored a -// misspelled key would resolve a contract nobody configured. +// decodeStrict rejects an unknown field and trailing data. A chain file that +// silently ignored a misspelled key would resolve a contract nobody configured. func decodeStrict(data []byte, v any) error { decoder := json.NewDecoder(bytes.NewReader(data)) decoder.DisallowUnknownFields() diff --git a/registry/resolve.go b/registry/resolve.go index f79be34..2903dff 100644 --- a/registry/resolve.go +++ b/registry/resolve.go @@ -40,8 +40,8 @@ func (e *MismatchError) Error() string { if e.Got == (common.Hash{}) { return fmt.Sprintf( "%s (chain %d): contract %s is recorded at %s, and that address holds no code. "+ - "The registry entry is stale, most likely because the chain was re-genesised. "+ - "Recorded code hash %s.", + "Either the entry is stale — a re-genesis is the usual cause — or the "+ + "endpoint this run read is behind the chain. Recorded code hash %s.", e.ChainName, e.ChainID, e.ContractName, e.Address, e.Want) } return fmt.Sprintf( From eff6aa18ef8d5e572fc0c28bbc433306964e0946 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 13:05:29 -0700 Subject: [PATCH 9/9] docs: sweep the registry feature's documentation for correctness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checked claims against behaviour rather than reading for typos. Six were wrong, and one was a code defect the sweep surfaced. The registry imports no sei-load package at all — `go list -deps` returns only itself. doc.go said "imports no other sei-load package except generator/bindings", which implies a dependency that does not exist, and boundary_test.go justified the allowance as "the bytecode the registry hashes against". It hashes what eth_getCode serves, not compiled bytecode, so nothing needs the bindings yet. Both now say the allowance is CDR-017's ceiling rather than a list of what is used. --dry-run previewed something a real run would not do. mockPrepareAll gave each instance its own random address while the live path groups by contract name and shares one, so a dry-run of a profile naming one scenario twice showed two contracts where a run gives one. It groups now. A preview that does not match is worse than no preview. generator/doc.go told an operator to "give each concurrent run its own deployer key until this is fixed". funder.Deployer returns the funding root, so no profile can do that — the advice reads as actionable and is not. It now says closing the gap needs a code change. Three stale names: deployPlanned for deployMissing in generator/doc.go, mockDeployAll for mockPrepareAll in StorageRW_test.go, and the mock-deploy paragraph describing per-instance binding. README's Command Line Options table gained --chain-file and --chain-record-path, including the two things an operator learns the hard way otherwise: a --chain-file path that does not exist fails startup, and recording needs genesisHash. Verified no phantom documentation: every flag named in a doc exists in main.go, and every JSON field named in a doc has a struct tag. Verifiers: gofmt, go vet, staticcheck, golangci-lint 0 issues, all 14 packages passing, git status clean. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 ++ generator/doc.go | 15 +++++++++------ generator/prepare.go | 24 ++++++++++++++---------- generator/scenarios/StorageRW_test.go | 2 +- registry/boundary_test.go | 10 ++++++---- registry/doc.go | 8 ++++---- 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 086ff3a..e762f7b 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,8 @@ Edit `my-config.json`: | `--track-blocks` | false | Track block statistics | | `--track-user-latency` | false | Track user latency metrics | | `--prewarm` | false | Prewarm accounts before test | +| `--chain-file` | | Contract registry file naming the target chain and its deployed contracts. Repeatable; each layers over the registry compiled into the binary, and a later file wins. A path that does not exist fails startup. | +| `--chain-record-path` | | Where to write a chain file describing what this run deployed, for an operator to review and commit. In a pod use `/dev/stdout`. Requires `genesisHash` in the profile. | ## Examples diff --git a/generator/doc.go b/generator/doc.go index ca20543..12a5efa 100644 --- a/generator/doc.go +++ b/generator/doc.go @@ -37,7 +37,7 @@ // # Deployment nonces // // A deployment leaves its nonce unset, so go-ethereum reads the deployer's -// pending nonce from the chain, and deployPlanned waits for the receipt before +// pending nonce from the chain, and deployMissing waits for the receipt before // it sends the next one. This is what makes a deployer with on-chain history safe: // the funding root has spent nonces before the run, and spends more right after // these deployments when it funds the pool. A nonce derived from the instance @@ -59,12 +59,15 @@ // workload nobody configured. // // Sequential runs on one key are safe, because the second reads a nonce the first -// advanced. Concurrent runs are not. Give each concurrent run its own deployer -// key until this is fixed. +// advanced. Concurrent runs are not, and a profile cannot avoid it: funder.Deployer +// returns the funding root, so an operator has no way to give two pods different +// deployer keys. Closing this needs a code change, not configuration. // // # Mock deploy // -// Under config.MockDeploy no deployment reaches a chain. Each instance binds at -// a random address against a nil backend, which is enough to shape calldata, and the -// deployer goes unused. This is the path --dry-run and the unit tests take. +// Under config.MockDeploy no deployment reaches a chain. Each contract gets one +// random address, shared by the instances that drive it exactly as a live run +// shares a resolved one, and the bind backend is nil, which is enough to shape +// calldata. The deployer goes unused. This is the path --dry-run and the unit +// tests take. package generator diff --git a/generator/prepare.go b/generator/prepare.go index 0725cc3..d33ce45 100644 --- a/generator/prepare.go +++ b/generator/prepare.go @@ -310,18 +310,22 @@ func logRegistrySource(reg *registry.Registry, chainID int64, genesisHash string // mockPrepareAll binds against a nil backend, which builds transactions and // never sends one. +// +// It groups by contract name the way the live path does, so a --dry-run of a +// profile naming one scenario twice previews the one shared contract that a real +// run would give it. func (g *generatorBuilder) mockPrepareAll() error { - for _, instance := range g.instances { - if err := instance.Scenario.Ready(g.config); err != nil { - return err - } - if binder := instance.Scenario.Binder(); binder != nil { - if err := binder(nil, types.NewAccount(false).Address); err != nil { - return err - } - } + bindings, err := groupByContractName(g.instances) + if err != nil { + return err } - return nil + if err := g.readyAll(); err != nil { + return err + } + for _, b := range bindings { + b.address = types.NewAccount(false).Address + } + return g.bindAll(nil, bindings) } // recordDeployments writes a chain file describing what this run deployed, for diff --git a/generator/scenarios/StorageRW_test.go b/generator/scenarios/StorageRW_test.go index 83d9e77..8cdb8e0 100644 --- a/generator/scenarios/StorageRW_test.go +++ b/generator/scenarios/StorageRW_test.go @@ -89,7 +89,7 @@ func TestStorageRWDeployAndGenerate(t *testing.T) { } // newAttachedStorageRW builds a StorageRW scenario from sc and attaches it at a -// known address under mock deploy, mirroring generator.mockDeployAll. It returns +// known address under mock deploy, mirroring generator.mockPrepareAll. It returns // the generator and a tx scenario carrying a funded sender. func newAttachedStorageRW(t *testing.T, sc config.Scenario) (scenarios.TxGenerator, *types.TxScenario) { t.Helper() diff --git a/registry/boundary_test.go b/registry/boundary_test.go index ed3b966..d57c22f 100644 --- a/registry/boundary_test.go +++ b/registry/boundary_test.go @@ -10,11 +10,13 @@ import ( // sei-load package; anything else is the standard library or a third party. const modulePath = "github.com/sei-protocol/sei-load" -// allowedInternalDeps are the sei-load packages the registry may import. +// allowedInternalDeps is the ceiling CDR-017 sets, not a list of what the +// registry uses. It imports none of these today. // -// generator/bindings holds the ABI and the bytecode the registry hashes against. -// Nothing else belongs here: reaching for config or types would tie the registry -// to the load generator, and CDR-017 exists to stop that. +// The contract bindings are on it because a later task may need an ABI here. The +// registry does not hash compiled bytecode — it hashes what eth_getCode serves — +// so nothing needs them yet. Nothing else belongs here: reaching for config or +// types would tie the registry to the load generator. var allowedInternalDeps = map[string]bool{ modulePath + "/generator/bindings": true, } diff --git a/registry/doc.go b/registry/doc.go index a0bd4e6..ea1fe0f 100644 --- a/registry/doc.go +++ b/registry/doc.go @@ -10,10 +10,10 @@ // // # Import boundary // -// This package imports no other sei-load package except generator/bindings. -// boundary_test.go asserts that. The rule is what keeps this package -// extractable: reaching for config or types would tie the registry to the load -// generator it exists to stay independent of. +// This package imports no sei-load package at all today. The contract bindings +// are the only one it may, and boundary_test.go asserts that ceiling. The rule +// is what keeps this package extractable: reaching for config or types would tie +// the registry to the load generator it exists to stay independent of. // // The one chain call this package makes is CodeReader.CodeAt. A caller supplies // an *ethclient.Client, or a test supplies a fake, so this package imports no