Skip to content

Latest commit

 

History

History
82 lines (58 loc) · 6.86 KB

File metadata and controls

82 lines (58 loc) · 6.86 KB

AGENTS.md — base/contracts

Working conventions for agents on this repo, distilled from prior sessions. These are defaults for new or touched work, not permission to churn unrelated code. When this file conflicts with the current tree, nearby code and enforced tests win.

Tooling

  • just snapshots — regenerates ABI, storage layout, and semver-lock in one shot. Run after source changes that affect API, storage, or semver snapshots.
  • just semver-lock — full source build + semver hash. Run before pushing semver-tagged src/ changes or when snapshots/semver-lock.json may change. Do not use just semver-lock-no-build after source edits; it reads existing artifacts and can write stale hashes.
  • just semver-lock-no-build — hash only, from existing artifacts.
  • CI regenerates snapshots/semver-lock.json and diffs it against the committed file. ABI/storage snapshots are generated by scripts/autogen, but check the current workflow before assuming CI diffs every snapshot type.
  • Snapshot generation is handled by Go scripts under scripts/autogen.
  • Tests: just test (full suite), just test --match-path <file> (single file), just test --match-test <name> (single test).
  • mise is used to keep local and CI Foundry versions in sync. Run mise install once after cloning.

Contract structure conventions

Style-guide ordering inside a contract

For new or touched contract bodies, prefer:

Type declarations → Constants → Immutables → State variables → Events → Errors → Constructor → external → external view → external pure → public → public view → public pure → internal → internal view → internal pure → private → private view → private pure.

Visibility

  • A concrete implementation not meant to be inherited should mark implementation internals private, not internal.
  • Inline single-caller and single-line helpers by default; extract only when logic is genuinely shared across multiple callers.

Interface / implementation split

  • Deploy-script-backed protocol implementations generally inherit base contracts and shared mixins (proxy-owned base, initializable, reinitializable, semver), not their own I<Name> interface.
  • When a contract has a paired interface, keep ABI-relevant declarations in sync with that interface. Do not invent a new interface only to satisfy this note.
  • The __constructor__() external declaration in interfaces is a tooling artifact for ABI/snapshot generation, not callable. Do not remove it.
  • For new semver functions in recent protocol work, prefer function version() public pure virtual returns (string memory).

API / event surface

  • Use explicit return x;, not named returns.
  • Trim events: drop params already carried in log metadata (e.g. block number).
  • Add indexed query getters when arbitrary entries need querying, not just tail reads.

Naming

  • Errors: prefer ContractName_ErrorName for new protocol contracts.
  • Events: no prefix (e.g. SomethingRegistered, TimestampSet).
  • Test contracts: ContractName_FunctionName_Test for standard contract behavior suites; standalone unit tests may use NameTest.
  • Test functions: prefer test_functionName_scenario_succeeds / _reverts for new tests.
  • Commits: conventional — type(scope): description (e.g. refactor(L1):, fix(L1):).
  • Branding: prefer offchain / onchain in new proof/protocol wording.

Access control

  • The proxy-owned base mixin provides _assertOnlyProxyAdminOwner() and _assertOnlyProxyAdminOrProxyAdminOwner().
  • For ProxyAdminOwnedBase, owner = proxyAdmin().owner(), read from the proxy-owner storage slot.
  • Secondary roles in proxy-owned protocol contracts are plain address fields with inline msg.sender checks. Inline one-line local role checks.

Proxy deployment

  • For fresh deployment of an initialized proxy, use upgradeToAndCall (atomic upgrade + init in one tx). Separate upgradeTo then initialize opens a window of uninitialized state; mutations in that window corrupt persistent state permanently. Separate upgrade-only paths can be valid for admin migrations, tests, or proxies that are not being initialized.
  • Upgradeable implementation constructors call _disableInitializers().
  • Contracts that inherit ReinitializableBase initialize with reinitializer(initVersion()). The reinitializer modifier fires before the function body, so the "already initialized" revert precedes any owner/admin assert — any caller triggers it. Contracts that use plain OpenZeppelin initializer are valid exceptions.

Adding a new Initializable contract

Standard deploy-script-backed src/ contracts with initialize() must be accounted for in the repo-wide reinitialization test that scans src/ via FFI. The test has explicit exclusions for categories such as L2/predeploys, periphery, and contracts with custom initialization state.

  • Deployed via the standard deploy script: add both <Name>Impl and <Name>Proxy entries. Proxy address comes from the shared test setup; impl address via the EIP-1967 implementation-slot helper on the proxy.
  • Not yet in the deploy script: add the source path to the excludes list (fixed capacity — mind the array size) with a comment.
  • Proxied-contract detection uses the @custom:proxied devdoc NatSpec tag — a proxied contract missing that tag won't get its proxy entry checked.
  • Always run forge test --match-test test_cannotReinitialize_succeeds when adding a standard initialized contract, in addition to the contract's own tests. A narrow --match-path run misses it.
  • New production contracts must be wired into the standard deploy script, not permanently excluded from the tracking test.

Test conventions

  • Tests for standard deploy-script-deployed system contracts should inherit the shared CommonTest base and use the deploy-script-deployed instances it exposes (via the shared Setup) — not new Contract() + a hand-rolled proxy. This makes the deploy script a real dependency of those tests.
  • Mirror an existing contract test of the same type: call super.setUp(), read parameters from the deploy config, and get the implementation via the EIP-1967 implementation-slot helper.
  • If an initializer edge-case test for a deploy-script-backed contract truly needs a fresh uninitialized proxy, deploy only a proxy and point it at the already-deployed implementation (via the implementation-slot helper). Do not new Contract() in that test file; that dodges the deploy-script dependency.
  • Re-initialize / initializer edge-case tests: reset the initializable slot with vm.store(addr, bytes32(0), bytes32(0)) behind a small helper, then prank the proxy admin. Skip on forks with skipIfForkTest(...).
  • Run the full suite after adding a contract, not just its own file (cross-cutting registration tests live outside the contract's file).
  • Kill redundant/duplicate tests: if an exact-value assertion subsumes a weaker one, keep one.