AutoGineer
Generated banner for Guidelines Advisor
Generated logo for Guidelines Advisor

Guidelines Advisor

Trail of BitsOfficialPublished by Trail of Bits

  • Skill
  • Review and audit
  • Security
  • CC-BY-SA-4.0
  • No issues found
Download ZIP

From the author

Smart contract development advisor based on Trail of Bits' best practices. Analyzes codebase to generate documentation/specifications, review architecture, check upgradeability patterns, assess implementation quality, identify pitfalls, review dependencies, and evaluate testing. Use when asking whether a smart contract project follows development best practices, reviewing on-chain/off-chain split, upgradeability, or delegatecall proxy patterns against guidelines, or seeking recommendations on contract design, inheritance, events, documentation, dependencies, or test strategy.
— the description by Trail of Bits

AutoGineer summary

Written by AutoGineer from the skill’s own files, not by the author

Guidelines Advisor provides a systematic analysis of smart contract projects based on established development guidelines. It evaluates codebase architecture, implementation quality, and testing strategies to generate documentation, identify potential pitfalls, and provide prioritized improvement recommendations. The advisor covers 11 assessment areas, including documentation, on-chain/off-chain computation, upgradeability, proxy patterns, function composition, inheritance, events, common pitfalls, dependencies, testing, and platform-specific guidance.

What it does

  • Generates plain English system descriptions and architectural documentation.
  • Reviews upgradeability patterns and delegatecall proxy implementations.
  • Assesses implementation quality including function composition, inheritance, and event logging.
  • Identifies common security pitfalls and evaluates dependency management.
  • Analyzes test suite coverage and suggests improvements.
  • Provides prioritized recommendations categorized by urgency.

When to use it

  • When evaluating if a smart contract project follows established development guidelines.
  • When reviewing the architecture of on-chain and off-chain components.
  • When assessing the security and implementation of upgradeability or proxy patterns.
  • When seeking actionable feedback on contract design, inheritance, or testing strategies.

Advantages

  • Provides project-specific findings with direct file and line references.
  • Systematically covers 11 distinct areas of smart contract development.
  • Offers a structured, prioritized roadmap for improvements.
  • Adapts analysis based on the specific platform and project context.

Usage examples

“Review my smart contract project for adherence to best practices regarding upgradeability and proxy patterns.”
The agent will analyze the codebase for delegatecall usage, storage layout consistency, and initialization patterns, providing a report on potential risks and improvements.
“Analyze my current test suite and suggest improvements for better coverage.”
The agent will evaluate existing test files, identify gaps in coverage, and recommend techniques like fuzzing or formal verification.

Install

  1. Download the skill as a ZIP:

    https://mp-staging.autogineer.com/agent-skills/trailofbits/guidelines-advisor/download

  2. Unzip it into the cross-client skills folder of your project:

    mkdir -p .agents/skills && unzip guidelines-advisor.zip -d .agents/skills

Documentation by Trail of Bits

Guidelines Advisor

Purpose

Systematically analyzes the codebase and provides guidance based on Trail of Bits' development guidelines:

  1. Generate documentation and specifications (plain English descriptions, architectural diagrams, code documentation)

  2. Optimize on-chain/off-chain architecture (only if applicable)

  3. Review upgradeability patterns (if your project has upgrades)

  4. Check delegatecall/proxy implementations (if present)

  5. Assess implementation quality (functions, inheritance, events)

  6. Identify common pitfalls

  7. Review dependencies

  8. Evaluate test suite and suggest improvements

Framework: Building Secure Contracts - Development Guidelines

How This Works

Phase 1: Discovery & Context

Explores the codebase to understand:

  • Project structure and platform

  • Contract/module files and their purposes

  • Existing documentation

  • Architecture patterns (proxies, upgrades, etc.)

  • Testing setup

  • Dependencies

Phase 2: Documentation Generation

Helps create:

  • Plain English system description

  • Architectural diagrams (using Slither printers for Solidity)

  • Code documentation recommendations (NatSpec for Solidity)

Phase 3: Architecture Analysis

Analyzes:

  • On-chain vs off-chain component distribution (if applicable)

  • Upgradeability approach (if applicable)

  • Delegatecall proxy patterns (if present)

Phase 4: Implementation Review

Assesses:

  • Function composition and clarity

  • Inheritance structure

  • Event logging practices

  • Common pitfalls presence

  • Dependencies quality

  • Testing coverage and techniques

Phase 5: Recommendations

Provides:

  • Prioritized improvement suggestions

  • Best practice guidance

  • Actionable next steps

Assessment Areas

I analyze 11 comprehensive areas covering all aspects of smart contract development. For detailed criteria, best practices, and specific checks, see ASSESSMENT_AREAS.md.

Quick Reference:

  1. Documentation & Specifications

    • Plain English system descriptions

    • Architectural diagrams

    • NatSpec completeness (Solidity)

    • Documentation gaps identification

  2. On-Chain vs Off-Chain Computation

    • Complexity analysis

    • Gas optimization opportunities

    • Verification vs computation patterns

  3. Upgradeability

    • Migration vs upgradeability trade-offs

    • Data separation patterns

    • Upgrade procedure documentation

  4. Delegatecall Proxy Pattern

    • Storage layout consistency

    • Initialization patterns

    • Function shadowing risks

    • Slither upgradeability checks

  5. Function Composition

    • Function size and clarity

    • Logical grouping

    • Modularity assessment

  6. Inheritance

    • Hierarchy depth/width

    • Diamond problem risks

    • Inheritance visualization

  7. Events

    • Critical operation coverage

    • Event naming consistency

    • Indexed parameters

  8. Common Pitfalls

    • Reentrancy patterns

    • Integer overflow/underflow

    • Access control issues

    • Platform-specific vulnerabilities

  9. Dependencies

    • Library quality assessment

    • Version management

    • Dependency manager usage

    • Copied code detection

  10. Testing & Verification

    • Coverage analysis

    • Fuzzing techniques

    • Formal verification

    • CI/CD integration

  11. Platform-Specific Guidance

    • Solidity version recommendations

    • Compiler warning checks

    • Inline assembly warnings

    • Platform-specific tools

For complete details on each area including what I'll check, analyze, and recommend, see ASSESSMENT_AREAS.md.

Example Output

When the analysis is complete, you'll receive comprehensive guidance covering:

  • System documentation with plain English descriptions

  • Architectural diagrams and documentation gaps

  • Architecture analysis (on-chain/off-chain, upgradeability, proxies)

  • Implementation review (functions, inheritance, events, pitfalls)

  • Dependencies and testing evaluation

  • Prioritized recommendations (CRITICAL, HIGH, MEDIUM, LOW)

  • Overall assessment and path to production

For a complete example analysis report, see EXAMPLE_REPORT.md.

Deliverables

I provide four comprehensive deliverable categories:

1. System Documentation

  • Plain English descriptions

  • Architectural diagrams

  • Documentation gaps analysis

2. Architecture Analysis

  • On-chain/off-chain assessment

  • Upgradeability review

  • Proxy pattern security review

3. Implementation Review

  • Function composition analysis

  • Inheritance assessment

  • Events coverage

  • Pitfall identification

  • Dependencies evaluation

  • Testing analysis

4. Prioritized Recommendations

  • CRITICAL (address immediately)

  • HIGH (address before deployment)

  • MEDIUM (address for production quality)

  • LOW (nice to have)

For detailed templates and examples of each deliverable, see DELIVERABLES.md.

Assessment Process

When invoked, I will:

  1. Explore the codebase

    • Identify all contract/module files

    • Find existing documentation

    • Locate test files

    • Check for proxies/upgrades

    • Identify dependencies

  2. Generate documentation

    • Create plain English system description

    • Generate architectural diagrams (if tools available)

    • Identify documentation gaps

  3. Analyze architecture

    • Assess on-chain/off-chain distribution (if applicable)

    • Review upgradeability approach (if applicable)

    • Audit proxy patterns (if present)

  4. Review implementation

    • Analyze functions, inheritance, events

    • Check for common pitfalls

    • Assess dependencies

    • Evaluate testing

  5. Provide recommendations

    • Present findings with file references

    • Ask clarifying questions about design decisions

    • Suggest prioritized improvements

    • Offer actionable next steps

Rationalizations (Do Not Skip)

RationalizationWhy It's WrongRequired Action
"System is simple, description covers everything"Plain English descriptions miss security-critical detailsComplete all 5 phases: documentation, architecture, implementation, dependencies, recommendations
"No upgrades detected, skip upgradeability section"Upgradeability can be implicit (ownable patterns, delegatecall)Search for proxy patterns, delegatecall, storage collisions before declaring N/A
"Not applicable" without verificationPremature scope reduction misses vulnerabilitiesVerify with explicit codebase search before skipping any guideline section
"Architecture is straightforward, no analysis needed"Obvious architectures have subtle trust boundariesAnalyze on-chain/off-chain distribution, access control flow, external dependencies
"Common pitfalls don't apply to this codebase"Every codebase has common pitfallsSystematically check all guideline pitfalls with grep/code search
"Tests exist, testing guideline is satisfied"Test existence ≠ test qualityCheck coverage, property-based tests, integration tests, failure cases
"I can provide generic best practices"Generic advice isn't actionableProvide project-specific findings with file:line references
"User knows what to improve from findings"Findings without prioritization = no action planGenerate prioritized improvement roadmap with specific next steps

Notes

  • I'll only analyze relevant sections (won't hallucinate about upgrades if not present)

  • I'll adapt to your platform (Solidity, Rust, Cairo, etc.)

  • I'll use available tools (Slither, etc.) but work without them if unavailable

  • I'll provide file references and line numbers for all findings

  • I'll ask questions about design decisions I can't infer from code

Ready to Begin

What I'll need:

  • Access to your codebase

  • Context about your project goals

  • Any existing documentation or specifications

  • Information about deployment plans

Let's analyze your codebase and improve it using Trail of Bits' best practices!

Files

resources/EXAMPLE_REPORT.md

Example Output

When the analysis is complete, you'll receive comprehensive guidance like this:

=== DEVELOPMENT GUIDELINES ANALYSIS ===

Project: NFT Marketplace
Platform: Solidity (Ethereum)
Analysis Date: March 15, 2024

---

## 1. DOCUMENTATION & SPECIFICATIONS

### Generated System Description

**Plain English Overview:**
The NFT Marketplace allows users to list, buy, and auction ERC721 tokens.
The system uses a decentralized orderbook where sellers create listings with
price and duration. Buyers can purchase instantly or place bids for auctions.
A 2.5% platform fee is collected on each sale.

**Key Assumptions:**
- All NFTs follow ERC721 standard
- Prices denominated in ETH only
- No token whitelisting (any ERC721 accepted)
- Platform fee immutable after deployment

### Architectural Diagrams Generated

✓ contract-interactions.png - Shows Marketplace, OrderBook, FeeCollector flow
✓ state-machine.png - Listing states (Created → Active → Sold/Cancelled/Expired)
✓ auction-flow.png - Bid placement and finalization sequence

### Documentation Gaps Identified

⚠ Missing NatSpec:
- OrderBook.cancelOrder() - No @notice or @param
- FeeCollector.withdrawFees() - Missing @dev implementation notes

⚠ Undocumented Assumptions:
- What happens if NFT transfer fails during purchase?
- Are listings automatically cleaned up after expiration?
- Fee distribution mechanism not explained

**Recommendation:** Add comprehensive NatSpec to all public functions
and document error handling for external calls.

---

## 2. ARCHITECTURE ANALYSIS

### On-Chain vs Off-Chain Components

**Current Distribution:**
- On-Chain: Listing creation, order execution, fee collection
- Off-Chain: Order discovery, price indexing, user notifications

**Optimization Opportunities:**
✓ Order matching is efficient (on-chain orderbook)
✗ Listing enumeration is gas-intensive

**Recommendation:**
Consider moving listing discovery off-chain using event indexing.
Keep core execution on-chain. Estimated gas savings: 40% for browse operations.

### Upgradeability Review

**Current Pattern:** TransparentUpgradeableProxy (OpenZeppelin)

**Assessment:**
✓ Proxy and implementation use shared storage base
✓ Initialization properly handled
✓ No function shadowing detected
✗ No timelock on upgrades (admin can upgrade immediately)

**Critical Issue:**
File: contracts/Marketplace.sol
The marketplace uses delegatecall proxy but admin is EOA without timelock.

**Recommendation:**
- Deploy TimelockController (48-hour delay)
- Transfer proxy admin to timelock
- Add emergency pause for critical bugs

### Proxy Pattern Security

**Findings:**
✓ Storage layout consistent (inherits MarketplaceStorage)
✓ No constructors in implementation
✓ Initialize function has initializer modifier
⚠ Immutable variables in proxy (PLATFORM_FEE)

**Issue:** PLATFORM_FEE defined as immutable in proxy will not update
if implementation changes this value.

**Fix:** Move PLATFORM_FEE to storage or accept it's immutable forever.

---

## 3. IMPLEMENTATION REVIEW

### Function Composition

**Complex Functions Identified:**
⚠ executePurchase() - 45 lines, cyclomatic complexity: 12
  - Handles payment, NFT transfer, fee calc, event emission
  - Recommendation: Extract _validatePurchase(), _processPayment(), _transferNFT()

⚠ finalizeAuction() - 38 lines, cyclomatic complexity: 10
  - Multiple nested conditionals for winner determination
  - Recommendation: Extract _determineWinner(), _refundLosers()

✓ Other functions well-scoped (average 15 lines)

### Inheritance

**Hierarchy Analysis:**

Marketplace ├─ Ownable ├─ ReentrancyGuard ├─ Pausable └─ MarketplaceStorage


✓ Shallow inheritance (depth: 2)
✓ No diamond problem
✓ Clear separation of concerns

**Slither Inheritance Graph:** contracts/inheritance.png (generated)

### Events

**Event Coverage:**
✓ 12 events defined
✓ All state changes emit events
✓ Consistent naming (ListingCreated, OrderFulfilled, BidPlaced)
✓ Indexed parameters for filtering (tokenId, seller, buyer)

⚠ Missing Events:
- Platform fee updates (if ever made variable)
- Pause/unpause operations

**Recommendation:** Add PlatformPaused/Unpaused events for monitoring.

### Common Pitfalls

**Issues Found:**

❌ CRITICAL: Reentrancy in executePurchase()
File: contracts/Marketplace.sol:234
```solidity
function executePurchase(uint256 listingId) external payable {
    Listing memory listing = listings[listingId];
    IERC721(listing.nftContract).transferFrom(listing.seller, msg.sender, listing.tokenId);
    // State update AFTER external call!
    listing.status = Status.Sold;
}

Fix: Follow checks-effects-interactions. Update state before external calls.

⚠ HIGH: Unvalidated external call return File: contracts/Marketplace.sol:245

payable(seller).transfer(amount);  // Can fail silently

Fix: Use call{value}() and check return value or use Address.sendValue().

✓ No timestamp dependence ✓ No tx.origin usage ✓ Integer overflow protected (Solidity 0.8+)

4. DEPENDENCIES

Current Dependencies: ✓ @openzeppelin/[email protected] - Well-tested, good choice ✗ Custom ERC721 implementation (contracts/CustomERC721.sol)

Issues: ⚠ CustomERC721 reinvents OpenZeppelin's ERC721

  • 234 lines of duplicate code

  • No added functionality

  • Increases audit surface

Recommendation: Replace CustomERC721 with OpenZeppelin's implementation. Saves 234 lines, reduces risk, improves maintainability.

Dependency Management: ✓ Using npm for dependencies ✓ Package versions pinned ⚠ Dependencies not updated in 8 months

Action: Update @openzeppelin/contracts to latest 5.x (breaking changes, test thoroughly)

5. TESTING EVALUATION

Current Test Suite:

  • 45 unit tests (forge test)

  • 12 integration tests

  • Coverage: 78%

Gaps Identified: ✗ No fuzzing (Echidna/Foundry) ✗ No formal verification ✗ Edge cases not covered:

  • Auction with zero bids

  • Listing with expired timestamp

  • Purchase during contract pause

Recommendations:

  1. Add Foundry invariant tests:

    • Total fees collected == sum of individual sales * 0.025

    • Active listings count matches actual active listings

    • No NFT can be in multiple active listings

  2. Increase coverage to 95%+ by testing:

    • Pausable functions during pause state

    • Reentrancy attack scenarios

    • Failed NFT transfers

  3. Add integration tests:

    • End-to-end auction flow with multiple bidders

    • Platform fee collection and withdrawal

    • Upgrade and data migration

Estimated Effort: 1-2 weeks to reach 95% coverage with invariant testing

PRIORITIZED RECOMMENDATIONS

CRITICAL (Fix Immediately)

  1. Fix reentrancy in executePurchase() [HIGH IMPACT]

    • Risk: Funds can be drained

    • Effort: 1 day

    • File: contracts/Marketplace.sol:234

  2. Validate external call returns [HIGH IMPACT]

    • Risk: Failed transfers not detected

    • Effort: 1 day

    • Files: Multiple payment operations

  3. Add timelock to upgrades [HIGH IMPACT]

    • Risk: Instant malicious upgrade

    • Effort: 2 days

HIGH (Before Mainnet)

  1. Remove CustomERC721, use OpenZeppelin [MEDIUM IMPACT]

    • Benefit: Reduce code, increase security

    • Effort: 3 days

  2. Increase test coverage to 95% [MEDIUM IMPACT]

    • Benefit: Catch edge case bugs

    • Effort: 1-2 weeks

  3. Add comprehensive NatSpec [LOW IMPACT]

    • Benefit: Better documentation

    • Effort: 2-3 days

MEDIUM (Post-Launch V2)

  1. Optimize listing enumeration [MEDIUM IMPACT]

    • Benefit: 40% gas savings on reads

    • Effort: 1 week

  2. Add invariant fuzzing [HIGH IMPACT]

    • Benefit: Discover hidden bugs

    • Effort: 1 week

SUMMARY

Overall Assessment: MODERATE MATURITY

The codebase follows many best practices with good use of OpenZeppelin libraries and clear architecture. Critical issues are reentrancy vulnerability and lack of upgrade timelock. Testing needs improvement.

Path to Production:

  1. Fix CRITICAL items (reentrancy, timelock) - Week 1

  2. Address HIGH items (dependencies, testing) - Week 2-3

  3. External audit - Week 4-5

  4. Mainnet deployment with documented limitations

  5. MEDIUM items in V2 - Month 2-3

Estimated Timeline: 3-4 weeks to production-ready state.

Analysis completed using Trail of Bits Development Guidelines

Requirements and compatibility

Works with: Claude Code, Claude apps, OpenAI Codex, GitHub Copilot, Cursor, Gemini CLI, Kiro, OpenCode, Goose, Amp, Roo Code, JetBrains Junie, Factory Droid, Devin (ex-Windsurf), Mistral Vibe, OpenHands, Letta

“Works with” means the tool reads the open Agent Skills format; a skill written for one tool may use features others lack.

Security

No issues found by our static scan· No issues found

A static scan is not a guarantee. Skills are third-party content: review a skill before you let an agent use it.

Attribution and licence

Published by
Trail of Bits
Publisher
Trail of Bits (Official)
Source
trailofbits/skills · plugins/building-secure-contracts/skills/guidelines-advisor · commit e4dd0bb
Licence
Creative Commons Attribution-ShareAlike 4.0 International (redistribution allowed)
  • LICENSE (added by AutoGineer from LICENSE)
  • ATTRIBUTION.md (written by AutoGineer)
Synced

AutoGineer is not affiliated with or endorsed by Trail of Bits.

Download ZIP

The ZIP holds the skill’s files, its licence files and an ATTRIBUTION.md. Download guidelines-advisor.zip