Remove formatting changes

README.md

@@ -43,7 +43,7 @@ using Library for bytes
 bytes._function()
 ```
 
-We could remedy this by insisting on the use public functions. However, developers may prefer internal functions because they are more gas efficient to call, due to how libraries are compiled in Solidity:
+Insisting on the use public functions could avoid this issue. However, we should not rely on this as a remedy as developers may prefer internal functions because they are more gas efficient to call, due to how libraries are compiled in Solidity:
 
 > ... the code of internal library functions that are called from a contract and all functions called from therein will at compile time be included in the calling contract, and a regular JUMP call will be used instead of a DELEGATECALL. ([source](https://docs.soliditylang.org/en/latest/contracts.html#libraries))
 
@@ -57,43 +57,45 @@ Custom errors should be used wherever possible (an exception could be introducin
 
 For example, `InsufficientBalance`.
 
-#### 3. Events names should be past tense.
+#### 3. Events
+
+##### A. Events names should be past tense.
 
 Events should track things that _happened_ and so should be past tense. Using past tense also helps avoid naming collisions with structs or functions.
 
 We are aware this does not follow precedent from early ERCs, like [ERC-20](https://eips.ethereum.org/EIPS/eip-20). However it does align with some more recent high profile Solidity, e.g. [1](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/976a3d53624849ecaef1231019d2052a16a39ce4/contracts/access/Ownable.sol#L33), [2](https://github.com/aave/aave-v3-core/blob/724a9ef43adf139437ba87dcbab63462394d4601/contracts/interfaces/IAaveOracle.sol#L25-L31), [3](https://github.com/ProjectOpenSea/seaport/blob/1d12e33b71b6988cbbe955373ddbc40a87bd5b16/contracts/zones/interfaces/PausableZoneEventsAndErrors.sol#L25-L41).
 
-YES:
-
-```solidity
-event OwnerUpdated(address newOwner);
-```
-
 NO:
 
 ```solidity
 event OwnerUpdate(address newOwner);
 ```
 
-#### 4. Event names should have `SubjectVerb` naming format.
-
 YES:
 
 ```solidity
 event OwnerUpdated(address newOwner);
 ```
 
+##### B. Prefer `SubjectVerb` naming format.
+
 NO:
 
 ```solidity
 event UpdatedOwner(address newOwner);
 ```
 
-#### 5. Avoid using assembly.
+YES:
+
+```solidity
+event OwnerUpdated(address newOwner);
+```
+
+#### 4. Avoid using assembly.
 
 Assembly code is hard to read and audit. We should avoid it unless the gas savings are very consequential, e.g. > 25%.
 
-#### 6. Avoid unnecessary named return arguments.
+#### 5. Avoid unnecessary named return arguments.
 
 In short functions, named return arguments are unnecessary.
 
@@ -113,91 +115,93 @@ function validate(UserOperation calldata userOp) external returns (bytes memory
 
 However, it is important to be explicit when returning early.
 
-YES:
+NO:
 
 ```solidity
 function validate(UserOperation calldata userOp) external returns (bytes memory context, uint256 validationData) {
   context = "";
   validationData = 1;
 
   if (condition) {
-    return (context, validationData);
+    return;
   }
 }
 ```
 
-NO:
+YES:
 
 ```solidity
 function validate(UserOperation calldata userOp) external returns (bytes memory context, uint256 validationData) {
   context = "";
   validationData = 1;
 
   if (condition) {
-    return;
+    return (context, validationData);
   }
 }
 ```
 
-#### 7. Prefer composition over inheritance.
+#### 6. Prefer composition over inheritance.
 
 If a function or set of functions could reasonably be defined as its own contract or as a part of a larger contract, prefer defining it as part of a larger contract. This makes the code easier to understand and audit.
 
 Note this _does not_ mean that we should avoid inheritance, in general. Inheritance is useful at times, most especially when building on existing, trusted contracts. For example, _do not_ reimplement `Ownable` functionality to avoid inheritance. Inherit `Ownable` from a trusted vendor, such as [OpenZeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/) or [Solady](https://github.com/Vectorized/solady).
 
-#### 8. Avoid writing interfaces.
+#### 7. Avoid writing interfaces.
 
 Interfaces separate NatSpec from contract logic, requiring readers to do more work to understand the code. For this reason, they should be avoided.
 
-#### 9. Avoid unnecessary version Pragma constraints.
+#### 8. Avoid unnecessary version Pragma constraints.
 
 While the main contracts we deploy should specify a single Solidity version, all supporting contracts and libraries should have as open a Pragma as possible. A good rule of thumb is to use the next major version. For example
 
 ```solidity
 pragma solidity ^0.8.0;
 ```
 
-#### 10. Default to declaring structs and errors within the interface, contract, or library where they are used.
+#### 9. Struct and Error Definitions
 
-This helps with clarity.
+##### A. Prefer declaring structs and errors within the interface, contract, or library where they are used.
 
-However, if a struct or error is used across many files, with no interface, contract, or library reasonably being the "owner," then define them in their own file. Multiple structs and errors can be defined together in one file.
+##### B. However, if a struct or error is used across many files, with no interface, contract, or library reasonably being the "owner," then define them in their own file. Multiple structs and errors can be defined together in one file.
 
-#### 11. Use named imports.
+#### 10. Imports
+
+##### A. Use named imports.
 
 Named imports help readers understand what exactly is being used and where it is originally declared.
 
-YES:
+NO:
 
 ```solidity
-import {Contract} from "./contract.sol"
+import "./contract.sol"
 ```
 
-NO:
+YES:
 
 ```solidity
-import "./contract.sol"
+import {Contract} from "./contract.sol"
 ```
 
 For convenience, named imports do not have to be used in test files.
 
-#### 12. Order imports alphabetically by file name.
+##### B. Order imports alphabetically (A to Z) by file name.
 
-YES:
+NO:
 
 ```solidity
-import {A} from './A.sol'
 import {B} from './B.sol'
+import {A} from './A.sol'
 ```
 
-NO:
+YES:
 
 ```solidity
-import {B} from './B.sol'
 import {A} from './A.sol'
+import {B} from './B.sol'
 ```
 
-#### 13. Group imports by external and local with a new line in between.
+##### C. Group imports by external and local with a new line in between.
 
 For example
 
@@ -217,7 +221,7 @@ import {MyHelper} from '../src/MyHelper.sol'
 import {Mock} from './mocks/Mock.sol'
 ```
 
-#### 14. Commenting to group sections of the code is permitted.
+#### 11. Commenting to group sections of the code is permitted.
 
 Sometimes authors and readers find it helpful to comment dividers between groups of functions. This permitted, however ensure the style guide [ordering of functions](https://docs.soliditylang.org/en/latest/style-guide.html#order-of-functions) is still followed.
 
@@ -233,40 +237,40 @@ For example
 /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
 ```
 
-#### 15. ASCII Art
+#### 12. ASCII Art
 
 ASCII art is permitted in the space between the end of the Pragmas and the beginning of the imports.
 
-#### 16. Prefer named arguments.
+#### 13. Prefer named arguments.
 
 Passing arguments to functions, events, and errors with explicit naming is helpful for clarity, especially when the name of the variable passed does not match the parameter name.
 
-YES:
+NO:
 
 ```
-pow({base: x, exponent: y, scalar: v})
+pow(x, y, v)
 ```
 
-NO:
+YES:
 
 ```
-pow(x, y, v)
+pow({base: x, exponent: y, scalar: v})
 ```
 
-#### 17. Prefer named parameters in mapping types.
+#### 14. Prefer named parameters in mapping types.
 
 Explicit naming parameters in mapping types is helpful for clarity, especially when nesting is used.
 
-YES:
+NO:
 
 ```
-mapping(address account => mapping(address asset => uint256 amount)) public balances;
+mapping(uint256 => mapping(address => uint256)) public balances;
 ```
 
-NO:
+YES:
 
 ```
-mapping(uint256 => mapping(address => uint256)) public balances;
+mapping(address account => mapping(address asset => uint256 amount)) public balances;
 ```
 
 ## 2. Development
@@ -306,6 +310,17 @@ contract TransferFromTest {
 
 This is generally good practice, but especially so because Forge does not give line numbers on assertion failures. This makes it hard to track down what, exactly, failed if a test has many assertions.
 
+NO:
+
+```solidity
+function test_transferFrom_works() {
+  // debits correctly
+  // credits correctly
+  // emits correctly
+  // reverts correctly
+}
+```
+
 YES:
 
 ```solidity
@@ -326,17 +341,6 @@ function test_transferFrom_reverts_whenAmountExceedsBalance() {
 }
 ```
 
-NO:
-
-```solidity
-function test_transferFrom_works() {
-  // debits correctly
-  // credits correctly
-  // emits correctly
-  // reverts correctly
-}
-```
-
 Note, this does not mean a test should only ever have one assertion. Sometimes having multiple assertions is helpful for certainty on what is being tested.
 
 ```solidity
@@ -349,47 +353,47 @@ function test_transferFrom_creditsTo() {
 
 #### 5. Use variables for important values in tests
 
-YES:
+NO:
 
 ```solidity
 function test_transferFrom_creditsTo() {
   assertEq(balanceOf(to), 0);
-  uint amount = 10;
-  transferFrom(from, to, amount);
-  assertEq(balanceOf(to), amount);
+  transferFrom(from, to, 10);
+  assertEq(balanceOf(to), 10);
 }
 ```
 
-NO:
+YES:
 
 ```solidity
 function test_transferFrom_creditsTo() {
   assertEq(balanceOf(to), 0);
-  transferFrom(from, to, 10);
-  assertEq(balanceOf(to), 10);
+  uint amount = 10;
+  transferFrom(from, to, amount);
+  assertEq(balanceOf(to), amount);
 }
 ```
 
 #### 6. Prefer fuzz tests.
 
 All else being equal, prefer fuzz tests.
 
-YES:
+NO:
 
 ```solidity
-function test_transferFrom_creditsTo(uint amount) {
+function test_transferFrom_creditsTo() {
   assertEq(balanceOf(to), 0);
+  uint amount = 10;
   transferFrom(from, to, amount);
   assertEq(balanceOf(to), amount);
 }
 ```
 
-NO:
+YES:
 
 ```solidity
-function test_transferFrom_creditsTo() {
+function test_transferFrom_creditsTo(uint amount) {
   assertEq(balanceOf(to), 0);
-  uint amount = 10;
   transferFrom(from, to, amount);
   assertEq(balanceOf(to), amount);
 }