Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions .shared/INTEGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,27 @@ node_modules/.bin/ts-node .shared/scripts/setup.ts --hooks
node_modules/.bin/ts-node .shared/scripts/setup.ts --hooks --include-pre-commit-hook # opt into the pre-commit hook
```

The pre-push hook executes the shared guardrail suite with `--fail-fast`, runs Prettier/ESLint/Solhint, and now
executes `npx hardhat test` by default. Set `SHARED_HARDHAT_PRE_PUSH_PRETTIER=0` or `SHARED_HARDHAT_PRE_PUSH_TEST=0`
to opt out temporarily, or override the test command with `SHARED_HARDHAT_PRE_PUSH_TEST_CMD="yarn test --runInBand"`.
Because the hook scripts live inside `.shared`, future updates to the tooling automatically flow into every
repository. Add `--include-pre-commit-hook` whenever you want the additional staged-file linting and compile guard that
the shared pre-commit hook provides.
The pre-push hook executes the shared guardrail suite with `--fail-fast`, runs Prettier/ESLint/Solhint, and
executes `npx hardhat test` by default. Because the hook scripts live inside `.shared`, future updates to the tooling
automatically flow into every repository. Add `--include-pre-commit-hook` whenever you want the additional
staged-file linting and compile guard that the shared pre-commit hook provides.

> **Heads-up:** The default pre-push flow now blocks on Prettier. Please fix formatting before pushing instead of
> suppressing the guard via `SHARED_HARDHAT_PRE_PUSH_PRETTIER=0`. A quick way to sweep the tree is:
>
> ```bash
> node_modules/.bin/ts-node .shared/scripts/linting/prettier.ts --write
> ```
>
> Only reach for the skip flag when you’re actively landing the formatter fix, and remember to re-enable it afterwards.

**Yarn Berry note:** the shared package expects the `node-modules` linker (`.yarnrc.yml` with `nodeLinker: node-modules`)
so that Node ambient typings and shared binaries resolve consistently. Switch existing repos with:

```bash
echo "nodeLinker: node-modules" > .yarnrc.yml
yarn install --mode=update-lockfile
```

> Prefer automation over manual flags? When you have access to this repository
> locally, `bash path/to/scripts/subtree/add.sh --help` prints the non-
Expand Down Expand Up @@ -218,6 +233,21 @@ node_modules/.bin/ts-node .shared/scripts/analysis/solhint.ts --quiet --max-warn
1. **Start clean** – abort if `git status --short` is non-empty. Fix compilation locally so guardrails have a stable baseline.
2. **Add the subtree** – prefer the wrapper: `bash scripts/subtree/add.sh --repo-url https://github.com/dtrinity/shared-hardhat-tools.git --branch main --prefix .shared`. Pass `--force-remove` only when replacing an existing directory after backing it up.
3. **Install the package** – `npm install file:./.shared` (or the equivalent `yarn/pnpm` command) so the bundled `ts-node` runtime is available. For Yarn Berry projects, follow immediately with `yarn install --mode=update-lockfile` to capture the new `file:` dependency so later `yarn install --immutable` checks succeed.

### Keeping subtrees fresh

1. Track the commit SHA applied to the repository (add it to the PR body or changelog).
2. Future updates become `git subtree pull --prefix=.shared https://github.com/dtrinity/shared-hardhat-tools.git <new-sha> --squash`.
3. Re-run `yarn install --mode=update-lockfile` (Yarn Berry) or the equivalent install command so lockfiles capture the new tree.
4. Re-run `node_modules/.bin/ts-node .shared/scripts/setup.ts --hooks --force` to make sure the latest hook wiring is present.
5. Run `node_modules/.bin/ts-node .shared/scripts/linting/prettier.ts --write` followed by `make lint`/`make test` to confirm the repo is still healthy.

### Rollout learnings

- **Prettier enforcement pays off.** Leaving `SHARED_HARDHAT_PRE_PUSH_PRETTIER` at its default forces teams to fix formatting once instead of repeatedly skipping it. Add the formatter sweep to your upgrade PR so reviewers only have to look at one big diff.
- **Ambient Hardhat/Node typings.** If you hit `TS2307` errors for built-in Node modules after updating the subtree, confirm that `types/ambient-hardhat.d.ts` and `types/node-globals.d.ts` are present and that your Yarn linker is set to `node-modules`.
- **Berry + subtrees.** Yarn 4 projects should keep `.yarnrc.yml` committed alongside the subtree to avoid accidental PnP installs, otherwise the shared scripts can’t locate ambient type definitions or the bundled `ts-node`.
- **Document the SHA.** Every shared update should call out the exact commit (`git subtree add/pull --prefix=.shared … <sha>`) in the PR description so future pulls can anchor to the same point.
4. **Run the setup preflight** – execute `node_modules/.bin/ts-node .shared/scripts/setup.ts --package-scripts` to add baseline npm scripts and surface missing prerequisites. Loop back with `--hooks`, `--configs`, or `--ci` once stakeholders sign off (teams sometimes stage those phases in their second pass, but do not forget them).
5. **Wire automation** – copy `.shared/ci/shared-guardrails.yml` into `.github/workflows/shared-guardrails.yml`, add a root `prettier.config.cjs` that re-exports `.shared/configs/prettier.config.cjs`, and stage both files with the rest of the integration diff so CI and local tooling stay in sync.
6. **Take a smoke-test lap** – run `npm run --prefix .shared lint:eslint -- --pattern 'hardhat.config.ts'`, `npm run --prefix .shared sanity:deploy-ids -- --quiet`, and either `npm run guardrails:check -- --skip-prettier --skip-solhint` (if the package script is wired up) or `npm run --prefix .shared guardrails:check -- --skip-prettier --skip-solhint` to confirm the shared tooling works in situ before committing. Drop the skips once formatting lands.
Expand Down
18 changes: 15 additions & 3 deletions .shared/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ SHARED_ENABLE_SLITHER_TARGETS ?= 1
SHARED_ENABLE_ROLES_TARGETS ?= 1

ROLES_SCAN_ARGS ?=
ROLES_GRANT_ARGS ?=
ROLES_TRANSFER_ARGS ?=
ROLES_REVOKE_ARGS ?=

Expand Down Expand Up @@ -148,7 +149,7 @@ roles.scan: ## Scan contracts for role assignments and ownership (make roles.sca
fi
@$(TS_NODE) $(SHARED_ROOT)/scripts/roles/scan-roles.ts --network "$(network)" --deployer "$(deployer)" --governance "$(governance)" $(if $(manifest),--manifest "$(manifest)",) $(ROLES_SCAN_ARGS)

roles.transfer: ## Transfer roles from deployer to governance (make roles.transfer network=network manifest=path [--yes])
roles.grant: ## Grant DEFAULT_ADMIN_ROLE to governance (make roles.grant network=network manifest=path [--dry-run] [--yes])
@if [ "$(network)" = "" ]; then \
echo "Must provide 'network' argument."; \
exit 1; \
Expand All @@ -157,7 +158,18 @@ roles.transfer: ## Transfer roles from deployer to governance (make roles.transf
echo "Must provide 'manifest' argument."; \
exit 1; \
fi
@$(TS_NODE) $(SHARED_ROOT)/scripts/roles/transfer-roles.ts --network "$(network)" --manifest "$(manifest)" $(if $(yes),--yes,) $(ROLES_TRANSFER_ARGS)
@$(TS_NODE) $(SHARED_ROOT)/scripts/roles/grant-default-admin.ts --network "$(network)" --manifest "$(manifest)" $(if $(yes),--yes,) $(ROLES_GRANT_ARGS)

roles.transfer: ## Transfer Ownable ownership to governance (make roles.transfer network=network manifest=path [--dry-run] [--yes])
@if [ "$(network)" = "" ]; then \
echo "Must provide 'network' argument."; \
exit 1; \
fi
@if [ "$(manifest)" = "" ]; then \
echo "Must provide 'manifest' argument."; \
exit 1; \
fi
@$(TS_NODE) $(SHARED_ROOT)/scripts/roles/transfer-ownership.ts --network "$(network)" --manifest "$(manifest)" $(if $(yes),--yes,) $(ROLES_TRANSFER_ARGS)

roles.revoke: ## Revoke deployer roles via Safe batch (make roles.revoke network=network manifest=path)
@if [ "$(network)" = "" ]; then \
Expand All @@ -181,4 +193,4 @@ endif
analyze.shared guardrails shared.update shared.setup \
shared.sanity.deploy-ids shared.sanity.deploy-clean shared.sanity.deploy-addresses shared.sanity.oracle-addresses \
shared.metrics.nsloc \
roles.scan roles.transfer roles.revoke
roles.scan roles.grant roles.transfer roles.revoke
41 changes: 24 additions & 17 deletions .shared/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,16 @@ import { runSlither } from '@dtrinity/shared-hardhat-tools/scripts/analysis/slit
const success = runSlither({ network: 'mainnet', failOnHigh: true });
```

### Manifest-Driven Role Transfers
### Manifest-Driven Role Governance

The shared runner migrates `Ownable` ownership and `DEFAULT_ADMIN_ROLE` by reading a manifest instead of bespoke scripts. Version 2 manifests default to auto-including every contract the deployer still controls and let you opt out with targeted exclusions or overrides. A minimal example:
The shared tooling now splits role/ownership hardening into focused scripts driven by a single manifest:

- `scan-roles` – read-only visibility into AccessControl and Ownable exposure.
- `grant-default-admin` – grants `DEFAULT_ADMIN_ROLE` to governance (direct execution).
- `revoke-roles` – prepares a Safe batch that revokes every deployer-held role.
- `transfer-ownership` – transfers `Ownable` contracts from the deployer to governance.

Version 2 manifests still auto-include every contract the deployer controls and let you opt out with exclusions or overrides. The schema is simpler—there is no renounce list or removal strategy anymore. A minimal manifest:

```json
{
Expand All @@ -216,10 +223,7 @@ The shared runner migrates `Ownable` ownership and `DEFAULT_ADMIN_ROLE` by readi
"autoInclude": { "ownable": true, "defaultAdmin": true },
"defaults": {
"ownable": { "newOwner": "{{governance}}" },
"defaultAdmin": {
"newAdmin": "{{governance}}",
"remove": { "strategy": "renounce", "execution": "direct", "address": "{{deployer}}" }
}
"defaultAdmin": { "newAdmin": "{{governance}}" }
},
"safe": {
"safeAddress": "0xSafe...",
Expand All @@ -235,8 +239,7 @@ The shared runner migrates `Ownable` ownership and `DEFAULT_ADMIN_ROLE` by readi
{
"deployment": "SpecialContract",
"defaultAdmin": {
"enabled": true,
"remove": { "strategy": "revoke", "execution": "safe", "address": "{{deployer}}" }
"enabled": true
}
}
]
Expand All @@ -246,27 +249,31 @@ The shared runner migrates `Ownable` ownership and `DEFAULT_ADMIN_ROLE` by readi
- `{{deployer}}` and `{{governance}}` placeholders resolve to the manifest addresses.
- `autoInclude` determines the default sweep; exclusions and overrides explicitly change the plan.
- `ownable.execution` must stay `direct`; Safe batches cannot call `transferOwnership`.
- Setting `remove.execution` to `safe` automatically switches to `revokeRole` and queues Safe transactions.
- The revoke script always generates `revokeRole` calls that the governance Safe executes offline.

Before running the CLI, add `roles.deployer` and `roles.governance` to your Hardhat network config. The shared scripts fall back to these values when the CLI flags are omitted, and refuse to run if neither source is provided.

Usage:

```bash
# Preview + execute direct ownership/admin transfers
ts-node .shared/scripts/roles/transfer-roles.ts --manifest manifests/roles.mainnet.json --network mainnet
# Scan for exposure and drift
ts-node .shared/scripts/roles/scan-roles.ts --manifest manifests/roles.mainnet.json --network mainnet --deployer 0xDeployer... --governance 0xGovernance... --drift-check

# Grant DEFAULT_ADMIN_ROLE directly (prompted unless --yes)
ts-node .shared/scripts/roles/grant-default-admin.ts --manifest manifests/roles.mainnet.json --network mainnet

# Preview + queue Safe revokeRole transactions only
# Queue Safe revokeRole transactions for every deployer-held role
ts-node .shared/scripts/roles/revoke-roles.ts --manifest manifests/roles.mainnet.json --network mainnet

# Dry-run without touching chain
ts-node .shared/scripts/roles/transfer-roles.ts --manifest manifests/roles.mainnet.json --network mainnet --dry-run-only
# Transfer Ownable contracts directly (prompted unless --yes)
ts-node .shared/scripts/roles/transfer-ownership.ts --manifest manifests/roles.mainnet.json --network mainnet

# Scan for new deployments and fail CI if coverage is missing
ts-node .shared/scripts/roles/scan-roles.ts --manifest manifests/roles.mainnet.json --network mainnet --deployer 0xDeployer... --governance 0xGovernance... --drift-check
# Add --dry-run to any script to print the plan without sending transactions
```

Each command performs a guarded dry-run first, printing the planned changes and listing any remaining non-admin roles so governance can follow up manually. Supply `--json-output report.json` to persist an execution summary alongside console output.
`--json-output report.json` persists an execution summary alongside console output. The Safe batch creator never signs
or submits transactions—it only prepares a tx-builder payload and SafeTx hash offline so governors can review before
proposing.

### Setting Up Git Hooks

Expand Down
Loading