# May-Lee's audit checklist
Golden rule: Talk early, talk often, take notes - audit audit audit!
At every stage, determine if there are tools that would have made the process easier.
## Estimate (5%)
- Size of the codebase
- how is the repo(s) divided
- type of code
- docs
- tests
- etc.
- loc/cloc/sloc (basically the same)
- TODO: Find a way to divide up a codebase by language and count loc that way
- Complexity of the codebase
- What's the stack?
- General architecture
- Different from common MVC/other patterns?
- Try to identify the architectural pattern
- How deep is team knowledge on the stack in question?
- Who are the SMEs and their expertise w/relation to the client concerns?
- What's your experience, personally?
- How well understood is the stack in use?
- How easy is the language to audit?
- Are the security properties/attack surface well-known?
- How well do you know these vulns?
- What are the non-deterministic processes?
## Kick-off (5%)
- Read the scope and look at the codebase
- Try to diagram the flow
- Look at one or two files/dirs you think might be relevant
- Come up with a list of good first questions
- Documentation
- Architecture
- What's the basic concept here?
- Could you explain this to someone?
- TODOs
- Anything that will be a "moving target"
- Client Concerns
- Keep this list top-of mind to address in the report later
- Ask about this at the kick-off
- Was it forked from/based off of anything else?
- Are there already known issues?
- Also, do you understand the general architecture?
## Set-up:
- LA repo is at scoped commit
- You have an audit tag short cut or force pushed the correct branch
- Create a local branch `audit-notes`
- You have the dependencies on your local environment installed
- Set you up some notes, where you talk in words about what's happening
- Have your automatic stuff setup (linters, etc.)
- Echidna and Slither for you young lady
- Ask if they can provide an implementation
## Audit Phase 1 (10%) - documentation, architecture, tree, client concerns set-up, basics
- Run static analysis, linters, check dependencies
- Decide who's going to focus on what, including yourself
- Match architecture/codebase with client concerns
- Use tree/other diagram to determine how the work wil be carried out
- Find the "common" issues for the stack
- XXS, reentrancy, over/underflow, etc.
## Audit Phase 2 (30%) - higher level/emergent effects/ecosystem impacts/
- Examine client concerns
- Build and do dynamic analysis
- Determine if there is a higher level emergent property
- Determine the following:
- What is it supposed to do
- What does it actually do
- What could it possibly also inadvertently do
## Audit Phase 3 (20%) - anything you couldn't do, you should communicate to your team
- What did you have trouble with (can also say this earlier)
- What is left to look at?
## Audit Phase 4 (20%) - initial audit report
- THIS IS THE GOAL:
- `ok done with my review, i've thoroughly examined every line and have some suggestions and maybe one issue or more on centralization concerns. but in general this is well written and simple enough that i feel confident that this is safe` -- Nathan Ginnever, 2021
- Don't start the report until:
- You understand the architecture
- You have examined if not every line, then most files
- Another thing is, it's going to be super unfun if you don't know what your colleagues are talking about
- Aim for having your first items as soon as code review is over
- Some people track issues as they come up
- You want it to feel like you're describing something that happened, as opposed to trying to craft the issues from thin air
- These are the three things to look for:
- Issues - classic vulns; clear exploits
- Suggestions - things that could improve the security of the project, but that aren't vulns necessarily (i.e., documentation)
- General comments (Any of these observations can later be turned into suggestions/issues, if needed)
- Documentation
- Testing
- Security-minded?
- Cite approved/"official" resources
## Audit Phase 5 (10%) - final audit report/verifications
- This is a huge pain in the ass
- After the IAR is turned in, the client is asked to make changes
- It's our job to explain the issues
- It requires a bit of memory here
- It's our job to verify that these issues have been properly dealt with
- Did the client's mitigation introduce new issues?
- It basically feels like reauditing bits of the code
# Project Estimation
## How many lines of code in what language does the project contain? (CLOC)
What is needed here:
Do it by each directory
Treasury, total: 6316
Treasury, src: 1859
Treasury, test: 3848
Multipool autoswap contracts: 1442
Contracts, total: 9
## How much test coverage is there?
I'm not seeing any for the AMM contracts
??What did anyone else see?
## What complexities does the code have?
In the Agoric Systems/ERTP/Zoe audit, it is mentioned that:
"Due to the requirements of defensive programming in the object capabilities model, the code style is essentially specific to Agoric."
So I would guess that if theoretically we had the same researchers (Dominic and Orange), that knocks 1 - 2 weeks off the auditing time.
## Is there anything else we need to consider to audit this project? (e.g. competencies we need, ...)
Understanding of liquidity pools and AMM mechanics in general
## Does this project look legitimate?
Yes, project looks legitimate and well-thought out.
## Can the work be separated in a different way?
## What other requirements should the project meet before we can start the audit? (e.g. more test coverage, more comments, …)
Documentation for users seems good and useful. But I would like documentation more focused on architecture or some kind of code walkthrough.
A visual representation of the architecture would be nice
## How long do you think the audit will take with how many auditors?
The first two weeks would be getting used to the DSL of the project.
Need at least: 3 weeks
Auditors:
2 - 3 with experience in Agoric
More than 4 if without
## Other resources found during the estimate process:
https://academy.shrimpy.io/post/what-is-an-amm-automated-market-maker
https://drive.google.com/file/d/1iuSMgL9BldLkQWI9ARXnoZRHmlb6UAzW/view
## Other comments/thoughts:
## (Optional) How much time did it take you approximately to estimate this project? Should we discuss this estimate in a meeting?
I tried to keep it under an hour. Most of the time I choose one part of a SOW that I feel confident about and respond to that; another researcher will usually then add their input/dispute the claim. Here I took about 90 minutes.
My own process notes:
Get size of the various types of directories
`git ls-files | xargs wc -l`
## Estimate (5%)
- Size of the codebase
- how is the repo(s) divided
- type of code
- docs
- tests
- etc.
- loc/cloc/sloc (basically the same)
- Complexity of the codebase
- What's the stack?
- General architecture
- Different from common MVC/other patterns?
- How deep is team knowledge on the stack in question?
- Who are the SMEs and their expertise w/relation to the client concerns?
- How well understood is the stack in use?
- How easy is the language to audit?
- Are the security properties/attack surface well-known?
- What are the non-deterministic processes?
# Solidity Notes
# Understanding NatSpec
###### (from [Solidity Documentation](https://docs.soliditylang.org/en/latest/natspec-format.html))
Ethereum Natural Language Specification Format
- Inspired by [Doxygen](https://en.wikipedia.org/wiki/Doxygen)
Solidity contracts SHOULD be fully annotated using NatSpec for all public interfaces (everything in the ABI)
Documentation (i.e., comments) MUST be inserted above each of the following, using the Doxygen notation format:
- `contract`
- `interface`
- `function`
- `event`
- `public` state variables are EQUIVALENT to `function` for the purposes of NatSpec
In Solidity, use:
- `///` for single or multi-line comments
- or `/**` ending with `*/`
Documentation output from the comments is segmented into developer-facing messages and end-user-facing messages.
- The Solidity compiler extracts these comments into a machine-readable format
- These messages then may be shown to the human at the time a Tx is signed
NOTE: Solidity has no intention to keep strict compatibility with Doxygen
## Tags
Tags are optional.
In the absence of tags, the Solidity compiler interprests `///` or `/**` as if tagged with `@notice`
Use multiple `@param` or `@return` statements in the case of multiple return values and parameters.
| Tag | Description | `contract` | `library` | `interface` | `function` | `public` state variable | `event` |
|---------------|------------------------------------------------------------------------------------|:----------:|:---------:|:-----------:|:-----------:|:-----------------------:|:-------:|
| `@title` | A title should describe the contract/interface | * | * | * | | | |
| `@author` | The name of the author | * | * | * | | | |
| `@notice` | Explain to an end user what this does | * | * | * | * | * | * |
| `@dev` | Explain to a developer any extra details | * | * | * | * | * | * |
| `@param` | Documents a parameter and must be followed by parameter name | | | | * | | * |
| `@return` | Documents the return variables of a contract's function | | | | * | * | |
| `@inheritdoc` | Copies all missing tags from base function (must be followed by the contract name) | | | | * | * | * |
| `@custom:` | Custom tag, semantics is application-defined (see below) | * | * | * | * | * | * |
### Custom tags
Annotations used by third-party tools can be accomplished using `@custom:`
- MUST be followed by one or more lowercase letters or hyphens
- CAN NOT start with a hyphen
- CAN be used everywhere and are part of the developer documentation
- A possible use case of this tag is analysis and verification tools
## Inheritance
Functions without NatSpec will automatically inherit the documentation of their base function, EXCEPT:
- When the parameter names are different
- When there is more than one base function
- When there is an explicit `@inheritdoc` tag
## Output
Generate documentation using
`solc --userdoc --devdoc ex1.sol`
Before Solidity 0.6.11, the documentation would produce two different JSON files: one for the end user as a notice when a function is executed and the other by the developer.
After Solidity 0.6.11, the output will also contain a `version` and a `kind` field. `version` is set to `1` but it is possible new versions might be introduced. `kind` must be EITHER `user` or `dev`
What is the output of the contract here:
<TODO: Link to contract>
Note that the key by which to find the methods is the function's canonical signature as defined in the [Contract ABI](https://docs.soliditylang.org/en/latest/abi-spec.html#abi-function-selector) and not the function's name.
## Other
Dynamic expressions: end-user client software may present JSON directly or apply pre-processing. Specifying dynamic expressions is outside of the scope of the Solidity documentation and more can be read here: https://github.com/aragon/radspec
At the time of this writing, the Solidity compiler only interprets tags if they are external or public. Comments used in internal and private functions will not be parsed.