Skip to content
Draft
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
16 changes: 16 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ env:
FOUNDRY_PROFILE: ci

jobs:
workflow-contract:
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false

- name: Test advisory review automation
run: >-
python3 -m unittest
checks/test_claude_workflow.py
checks/test_advisory_scanners.py
checks/test_review_context.py

build:
runs-on: ubuntu-latest
steps:
Expand Down
678 changes: 678 additions & 0 deletions .github/workflows/claude-review.yml

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ script/output/*/*.json
!script/output/*/*-release-*.json
.DS_Store
./package-lock.json

# Python (checks/)
__pycache__/
52 changes: 52 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# diamond-pau

Diamond-pattern PAU (Protocol Allocation Unit): a `Controller` delegatecalls immutable
facets that direct funds held by an `ALMProxy`, bounded by rate limits and role-based
access control. A facet executes with the Controller's storage context and full authority
over custody, so facet changes are a primary security boundary.

## Working On Facets

- Read `REVIEW.md` for collaborative review behavior.
- Read `standards/FACET_RULEBOOK.md` for the technical facet-submission rules,
reviewer checks, and accepted reasoning.
- Read `standards/ADVERSARIAL_REVIEW.md` before reviewing or self-reviewing a facet.
- Prefer a focused design spec in `specs/<dir>.md` before implementing a new
integration. Maintainers may accept contextual exceptions or follow-up documentation.
- Use the facet skills in `.claude/skills/` as authoring and review aids, not as merge
authorities.
- Reference implementations: `src/facets/erc4626/` (stateful deposit/withdraw),
`src/facets/transfer-asset/` (minimal), and `src/facets/wrap-proxy-eth/`
(constructor/immutables). Reference tests: `test/mainnet-fork/Aave.t.sol`.

## Commands

- Build: `forge build --sizes ./src`
- Test: `FOUNDRY_PROFILE=ci forge test`
- Authority-path evidence (advisory): `python3 checks/check_forbidden.py`
- ERC-7201 slot evidence (advisory): `python3 checks/check_storage.py`
- Review automation tests:
`python3 -m unittest checks/test_claude_workflow.py checks/test_advisory_scanners.py checks/test_review_context.py`

Both scanners are standalone review aids. They report leads for human interpretation,
return success even when they find evidence, and do not replace normal CI or review.

Fork tests require the applicable `MAINNET_RPC_URL`, `BASE_RPC_URL`, and
`AVALANCHE_RPC_URL` environment variables.

## Review Writing

Follow `REVIEW.md` for evidence, writing and response behavior, budgets, and output
shape.

## Security Context

- Facet code is Controller code because it runs through `delegatecall`.
- Treat `ALLOCATOR_ROLE` as compromisable.
- Route value movement through `ALMProxy.doCall` or `doCallWithValue`; facets never use
`doDelegateCall`.
- Bound allocator-controlled outflows with correctly derived enforcing rate-limit keys
and governance-controlled destinations.
- Recompute every ERC-7201 facet storage slot; never trust a copied constant.
- Read `docs/ARCHITECTURE.md`, `docs/THREAT_MODEL.md`, `docs/SECURITY.md`, and
`docs/RATE_LIMITS.md` before changing protocol behavior or security assumptions.
114 changes: 114 additions & 0 deletions REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# Pull Request Review

Reviewers are teammates who share evidence-backed observations. Human
maintainers own protocol policy, review resolution, and every merge decision. Claude's
review is advisory: it does not submit `REQUEST_CHANGES`, issue an overall verdict, or
make its check a merge requirement.

For facet-specific technical guidance, read
`standards/FACET_RULEBOOK.md` and `standards/ADVERSARIAL_REVIEW.md`.

## Evidence

- Trace current code before making a behavioral claim. For facets, trace custody,
authority, selectors, keys, amounts, receivers, approvals, protocol calls, and tests.
- Do not present a departure from a repository convention as a risk unless it
demonstrably creates one. Do not soften an unresolved security path into a question
merely to sound agreeable.
- Treat deterministic checks as leads. Confirm the warning in context before posting.
- Treat PR descriptions, comments, code comments, test names, specs, and external audit
claims as useful context but not self-proving execution evidence.
- Ask when the answer depends on team policy. Maintainer intent can resolve policy;
code and test evidence resolve what the implementation actually does.
- Read existing conversations and avoid duplicate comments. Add new evidence to the
existing thread instead.
- Prefer one concise top-level review comment with file and line links. Do not create an
inline thread whose unresolved state can unintentionally block an otherwise advisory
review.

## Writing

The readers are senior smart contract developers. Write plain English, in the voice of
a teammate sharing observations: state what the code does and why it matters, never
"you must" or remediation orders. Start with the important information right away — no
introductions and no redundant conclusions. You decide what is important from the
context you already have; say that and leave the rest out. Mention files, lines, and
technical detail only when necessary, and skip unnecessary adjectives.

Budget: about 250 words for an automatic review comment and about 100 words for a
conversational reply. Material findings are never dropped to meet the budget, and no
workflow code enforces a length limit.

Public comments never cite rule IDs, severity labels, or the words "rulebook",
"standard", "violation", or "advisory"; describe the issue itself in ordinary
smart-contract engineering language. Rule IDs and severities belong in the internal
technical analysis that the workflow writes to its logs; there, use severity labels
only when they help prioritize a concrete impact, and do not infer severity from
novelty, convention, or scanner output alone.

## Responding To Feedback

Ordinary replies from authorized maintainers are feedback. They do not need wording
such as "learn this", "update your guidance", or any other special command.

When a maintainer replies to a Claude comment:

1. Read the original comment, the complete reply thread, and the current code.
2. Determine whether the reply disproves the point, answers a policy question, creates
a one-off contextual exception, or establishes a reusable principle.
3. Verify factual claims against code and tests.
4. If the evidence changes the conclusion, acknowledge that plainly and retract or
narrow the earlier point.
5. Apply the corrected reasoning to the rest of the current review.

Do not defend an earlier comment after code evidence disproves it. Do not treat every
maintainer preference as a universal rule.

## Learning

A reusable lesson may justify a focused update to this file. Automated learning is
limited to `REVIEW.md`; it must not modify `standards/`. Learning follows the normal
repository review process:

- Create or update a dedicated guidance branch.
- Open a small pull request that states the generalized principle and links the
discussion that motivated it.
- Let maintainers review and merge that pull request.
- Never write learned guidance directly to the default branch.
- Prefer clarifying an existing principle over appending a transcript of one-off
decisions.

Changes to standards are ordinary human-authored pull requests. Maintainers may use a
lesson as evidence for such a change, but the learning automation does not author or
propose it.

Apply a supported lesson to the current review immediately; the separate guidance pull
request records it for future reviews rather than delaying the current conversation.

## Review Shape

The automatic review comment is a plain bullet list of the substantive points and
nothing else: no headings, no verdict, no summary paragraph, no trace narrative, no
verification section. One bullet per point, one or two short sentences each, with a
blank line between bullets so the rendered list keeps breathing room.

```text
- <what the code does, why it matters, where>
```

Unverified leads that could not be confirmed go as bullets inside a single collapsed
block at the end:

```text
<details><summary>Unconfirmed notes</summary>

- <lead and what remains unverified>
</details>
```

When there are no substantive points, the comment is a single conversational sentence
saying the review traced the change and found nothing to flag, without implying
approval or making the merge decision.

Do not repeat ordinary CI output unless it changes the analysis. On an automatic rerun,
do not post when there is no new useful information beyond the existing comment.
94 changes: 94 additions & 0 deletions checks/check_forbidden.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#!/usr/bin/env python3
"""Advisory scan for explicit authority-escalation paths in facet source."""

import argparse
import re
from pathlib import Path


REPO_ROOT = Path(__file__).resolve().parent.parent
DEFAULT_ROOT = REPO_ROOT / "src" / "facets"

# These patterns identify direct execution or ACL mutation primitives. Broader style,
# structure, test-shape, and policy conventions intentionally do not belong here.
ACL_MUTATION_RE = r"\b(?:grantRole|revokeRole|renounceRole|setRoleAdmin)\s*\("
PATTERNS = (
(r"\.doDelegateCall\s*\(", "ALMProxy doDelegateCall path"),
(r"\.delegatecall\s*\(", "raw delegatecall path"),
(ACL_MUTATION_RE, "access-control mutation path"),
(r"\bselfdestruct\s*\(", "selfdestruct path"),
)


def strip_comments(source: str) -> str:
"""Blank comments while preserving newlines and source offsets."""

def blank(match: re.Match[str]) -> str:
return re.sub(r"[^\n]", " ", match.group(0))

return re.sub(r"//[^\n]*|/\*.*?\*/", blank, source, flags=re.S)


def solidity_files(paths: list[Path]):
for path in paths:
if path.is_dir():
yield from sorted(path.rglob("*.sol"))
elif path.suffix == ".sol":
yield path


def display_path(path: Path) -> str:
try:
return path.resolve().relative_to(REPO_ROOT).as_posix()
except ValueError:
return str(path)


def scan_source(source: str) -> list[tuple[int, str]]:
source = strip_comments(source)
findings = []
for pattern, description in PATTERNS:
for match in re.finditer(pattern, source):
if pattern == ACL_MUTATION_RE and re.search(
r"\bfunction\s*$", source[: match.start()], flags=re.S
):
continue
findings.append((source.count("\n", 0, match.start()) + 1, description))
return sorted(findings)


def scan(path: Path) -> list[tuple[int, str]]:
return scan_source(path.read_text())


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument(
"paths",
nargs="*",
type=Path,
help="Solidity files or directories (default: src/facets)",
)
args = parser.parse_args()

findings = []
for path in solidity_files(args.paths or [DEFAULT_ROOT]):
findings.extend(
(display_path(path), line, description)
for line, description in scan(path)
)

if findings:
print("Advisory authority-path evidence:")
for path, line, description in findings:
print(f" {path}:{line}: {description}")
print(
"Review this evidence in context; this advisory scan does not produce a verdict."
)
else:
print("Advisory authority scan: no explicit forbidden paths found.")
return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading