docs: use natspec smells (#653)

* refactor/repo-structure

* chore: add natspec-smells and config

* refactor: remove tests from old strategies

* fix: name

* fix: remove yarn log

* fix: format

* docs: add return param poolId

* chore: ignore deprecated strategies

* fix: package.json and foundry.toml

* docs: fix and add missing natspec

* chore: fix format

* docs: fix natstpec

* chore: lock netspec-smells version

* docs: typo

* chore: update bun.lockb

* chore: add natspec linting to package.json scripts

* chore: add natspec linting to github workflow

* docs: comply with natspec-smells

---------

Co-authored-by: OneTony <onetony@defi.sucks>

.github/workflows/forge-test.yml

@@ -34,13 +34,15 @@ jobs:
         id: smock
 
       - name: Run Forge Format
-        run: |
-          forge fmt --check
+        run: forge fmt --check
         id: fmt
 
+      - name: Run natspec linting
+        run: bun lint:natspec
+        id: natspec
+
       - name: Run Forge build
-        run: |
-          forge compile
+        run: forge compile
         id: build
 
       - name: Run Forge test

contracts/core/Allo.sol

@@ -191,6 +191,7 @@ contract Allo is IAllo, Native, Initializable, Ownable, AccessControlUpgradeable
     /// @param _managers The managers of the pool
     /// @custom:initstrategydata The encoded data will be specific to a given strategy requirements,
     ///    reference the strategy implementation of 'initialize()'
+    /// @return poolId The ID of the pool
     function createPool(
         bytes32 _profileId,
         address _strategy,
@@ -693,6 +694,7 @@ contract Allo is IAllo, Native, Initializable, Ownable, AccessControlUpgradeable
     }
 
     /// @dev Logic copied from ERC2771ContextUpgradeable OZ contracts
+    /// @return the sender of the call
     function _msgSender() internal view virtual override returns (address) {
         uint256 calldataLength = msg.data.length;
         if (isTrustedForwarder(msg.sender) && calldataLength >= 20) {
@@ -703,6 +705,7 @@ contract Allo is IAllo, Native, Initializable, Ownable, AccessControlUpgradeable
     }
 
     /// @dev Logic copied from ERC2771ContextUpgradeable OZ contracts
+    /// @return calldata filtering the sender address when the trusted forward is the operator
     function _msgData() internal view override returns (bytes calldata) {
         uint256 calldataLength = msg.data.length;
         if (isTrustedForwarder(msg.sender) && calldataLength >= 20) {
@@ -777,6 +780,8 @@ contract Allo is IAllo, Native, Initializable, Ownable, AccessControlUpgradeable
     }
 
     /// @dev Logic copied from ERC2771ContextUpgradeable OZ contracts
+    /// @param forwarder address to check if it is trusted
+    /// @return true if it is trusted, false otherwise
     function isTrustedForwarder(address forwarder) public view returns (bool) {
         return forwarder == _trustedForwarder;
     }

contracts/core/interfaces/IAllo.sol

@@ -31,6 +31,12 @@ interface IAllo {
     /// ======================
 
     /// @notice the Pool struct that all strategy pools are based from
+    /// @param profileId The profile id
+    /// @param strategy The strategy address
+    /// @param token The address of the token used by the pool
+    /// @param metadata The 'Metadata' of the pool
+    /// @param managerRole id of the manager role
+    /// @param adminRole id of the admin role
     struct Pool {
         bytes32 profileId;
         IBaseStrategy strategy;
@@ -150,6 +156,7 @@ interface IAllo {
     /// @param _managers The managers of the pool
     /// @custom:initstrategydata The encoded data will be specific to a given strategy requirements,
     ///    reference the strategy implementation of 'initialize()'
+    /// @return poolId The ID of the pool
     function createPool(
         bytes32 _profileId,
         address _strategy,
@@ -218,6 +225,7 @@ interface IAllo {
     /// @param _poolId The ID of the pool to register the recipient for
     /// @param _recipientAddresses The addresses of the recipients to register
     /// @param _data The data to pass to the strategy and may be handled differently by each strategy
+    /// @return array of recipients ids registered
     function registerRecipient(uint256 _poolId, address[] memory _recipientAddresses, bytes memory _data)
         external
         payable
@@ -227,6 +235,7 @@ interface IAllo {
     /// @param _poolIds The pool ID's to register the recipients for
     /// @param _recipientAddresses The addresses of the recipients to register
     /// @param _data The data to pass to the strategy and may be handled differently by each strategy
+    /// @return array of arrays of recipients ids registered in each pool
     function batchRegisterRecipient(
         uint256[] memory _poolIds,
         address[][] memory _recipientAddresses,
@@ -240,7 +249,7 @@ interface IAllo {
     /// @param _amount The amount to fund the pool with
     function fundPool(uint256 _poolId, uint256 _amount) external payable;
 
-    /// @notice Allocates funds to a recipient.
+    /// @notice Allocates funds to recipients.
     /// @dev Each strategy will handle the allocation of funds differently.
     /// @param _poolId The ID of the pool to allocate funds from
     /// @param _recipients The recipients to allocate to
@@ -250,8 +259,14 @@ interface IAllo {
         external
         payable;
 
-    /// @notice Allocates funds to multiple recipients.
-    /// @dev Each strategy will handle the allocation of funds differently
+    /// @notice Allocates to multiple pools
+    /// @dev The encoded data will be specific to a given strategy requirements, reference the strategy
+    ///      implementation of allocate().
+    /// @param _poolIds IDs of the pools
+    /// @param _recipients Addresses of the recipients
+    /// @param _amounts Amounts to allocate to each recipient
+    /// @param _values amounts of native tokens to allocate for each pool
+    /// @param _datas encoded data unique to the strategy for that pool
     function batchAllocate(
         uint256[] calldata _poolIds,
         address[][] calldata _recipients,

contracts/core/interfaces/IRegistry.sol

@@ -36,6 +36,12 @@ interface IRegistry {
     /// ======================
 
     /// @dev The Profile struct that all profiles are based from
+    /// @param id The profile id
+    /// @param nonce The nonce used to generate the profile
+    /// @param name The name of the profile
+    /// @param metadata The 'Metadata'
+    /// @param owner The 'owner' address of the profile
+    /// @param anchor The lastest 'anchor' address generated for this profile
     struct Profile {
         bytes32 id;
         uint256 nonce;
@@ -50,20 +56,35 @@ interface IRegistry {
     /// ======================
 
     /// @dev Emitted when a profile is created. This will return your anchor address.
+    /// @param profileId The 'profileId' of the new profile
+    /// @param nonce The nonce used to generate the 'anchor' address
+    /// @param name The name used to generate the 'anchor' address
+    /// @param metadata The 'Metadata' used to generate the 'anchor' address
+    /// @param owner The 'owner' used to generate the 'anchor' address
+    /// @param anchor The 'anchor' address generated
     event ProfileCreated(
         bytes32 indexed profileId, uint256 nonce, string name, Metadata metadata, address owner, address anchor
     );
 
     /// @dev Emitted when a profile name is updated. This will update the anchor when the name is updated and return it.
+    /// @param profileId The 'profileId' of the updated profile
+    /// @param name The new name used to generate the 'anchor' address
+    /// @param anchor The new 'anchor' address generated
     event ProfileNameUpdated(bytes32 indexed profileId, string name, address anchor);
 
     /// @dev Emitted when a profile's metadata is updated.
+    /// @param profileId The 'profileId' of the updated profile
+    /// @param metadata The new 'Metadata'
     event ProfileMetadataUpdated(bytes32 indexed profileId, Metadata metadata);
 
     /// @dev Emitted when a profile owner is updated.
+    /// @param profileId The 'profileId' of the updated profile
+    /// @param owner The new 'owner'
     event ProfileOwnerUpdated(bytes32 indexed profileId, address owner);
 
     /// @dev Emitted when a profile pending owner is updated.
+    /// @param profileId The 'profileId' of the updated profile
+    /// @param pendingOwner The address of the pending owner
     event ProfilePendingOwnerUpdated(bytes32 indexed profileId, address pendingOwner);
 
     /// =========================

contracts/core/libraries/Clone.sol

@@ -24,9 +24,10 @@ import {ClonesUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/pr
 /// @notice A helper library to create deterministic clones of the strategy contracts when a pool is created
 /// @dev Handles the creation of clones for the strategy contracts and returns the address of the clone
 library Clone {
-    /// @dev Create a clone of the contract
+    /// @notice Create a clone of the contract
     /// @param _contract The address of the contract to clone
     /// @param _nonce The nonce to use for the clone
+    /// @return address of the new contract
     function createClone(address _contract, uint256 _nonce) internal returns (address) {
         bytes32 salt = keccak256(abi.encodePacked(msg.sender, _nonce));
 

contracts/core/libraries/Errors.sol

@@ -115,6 +115,7 @@ contract Errors {
     error REGISTRATION_ACTIVE();
 
     /// @notice Thrown when there is an error in recipient.
+    /// @param recipientId The recipient id associated to the error.
     error RECIPIENT_ERROR(address recipientId);
 
     /// @notice Thrown when the allocation is not active.

contracts/core/libraries/Transfer.sol

@@ -45,6 +45,10 @@ library Transfer {
     }
 
     /// @notice Stores the permit2 data for the allocation
+    /// @param permit2 The address of the permit2 contract
+    /// @param nonce The nonce of the permit
+    /// @param deadline The deadline timestamp of the permit
+    /// @param signature The signature
     struct PermitData {
         ISignatureTransfer permit2;
         uint256 nonce;
@@ -136,6 +140,9 @@ library Transfer {
     /// @notice Splits a signature into its r, s, v components
     /// @dev compact EIP-2098 signatures are accepted as well
     /// @param _signature The signature
+    /// @return r
+    /// @return s
+    /// @return v
     function _splitSignature(bytes memory _signature) internal pure returns (bytes32 r, bytes32 s, uint8 v) {
         if (_signature.length == 65) {
             assembly {

contracts/factories/ContractFactory.sol

@@ -38,6 +38,8 @@ contract ContractFactory {
     /// ======================
 
     /// @notice Emitted when a contract is deployed.
+    /// @param deployed address
+    /// @param salt used in contract creation
     event Deployed(address indexed deployed, bytes32 indexed salt);
 
     /// ======================

contracts/factories/DGLFactory.sol

@@ -72,6 +72,7 @@ contract DGLFactory is Ownable {
     }
 
     /// @notice Creates a new DirectGrantsLiteStrategy
+    /// @return strategy The address of the new strategy
     function createStrategy() external returns (address strategy) {
         strategy = address(new DirectGrantsLiteStrategy(allo, name));
         emit StrategyCreated(strategy);
@@ -80,6 +81,7 @@ contract DGLFactory is Ownable {
     /// @notice Creates a new DirectGrantsLiteStrategy with custom params
     /// @param _allo The 'Allo' contract
     /// @param _name The name of the strategy
+    /// @return strategy The address of the new strategy
     function createCustomStrategy(address _allo, string memory _name) external returns (address strategy) {
         strategy = address(new DirectGrantsLiteStrategy(_allo, _name));
         emit StrategyCreated(strategy);

contracts/factories/DVMDTFactory.sol

@@ -88,6 +88,7 @@ contract DVMDTFactory is Ownable {
     }
 
     /// @notice Creates a new DonationVotingMerkleDistributionDirectTransferStrategy
+    /// @return strategy The address of the new strategy
     function createStrategy() external returns (address strategy) {
         strategy = address(new DonationVotingMerkleDistributionDirectTransferStrategy(allo, name, permit2));
         emit StrategyCreated(strategy);
@@ -97,6 +98,7 @@ contract DVMDTFactory is Ownable {
     /// @param _allo The 'Allo' contract
     /// @param _name The name of the strategy
     /// @param _permit2 The permit2 contract
+    /// @return strategy The address of the new strategy
     function createStrategyCustom(address _allo, string memory _name, ISignatureTransfer _permit2)
         external
         returns (address strategy)

contracts/strategies/examples/direct-allocation/DirectAllocation.sol

@@ -85,14 +85,12 @@ contract DirectAllocationStrategy is BaseStrategy, Native, Errors {
         if (msg.value < _totalNativeAmount) revert ETH_MISMATCH();
     }
 
-    /// @notice Distribute funds to recipients
-    /// @dev This function is not implemented
+    /// @inheritdoc BaseStrategy
     function _distribute(address[] memory, bytes memory, address) internal virtual override {
         revert NOT_IMPLEMENTED();
     }
 
-    /// @notice Register a recipient
-    /// @dev This function is not implemented
+    /// @inheritdoc BaseStrategy
     function _register(address[] memory, bytes memory, address) internal virtual override returns (address[] memory) {
         revert NOT_IMPLEMENTED();
     }

contracts/strategies/examples/donation-voting/DonationVotingMerkleDistribution.sol

@@ -53,13 +53,18 @@ contract DonationVotingMerkleDistribution is DonationVotingOffchain {
     error MERKLE_ROOT_NOT_SET();
 
     /// @notice Thrown when distribution is attempted twice for the same 'index'
+    /// @param _index The index for which a repeated distribution was attempted
     error ALREADY_DISTRIBUTED(uint256 _index);
 
     /// ================================
     /// ========== Struct ==============
     /// ================================
 
     /// @notice Stores the details of the distribution.
+    /// @param index The index in the merkle tree
+    /// @param recipientId The id of the recipient
+    /// @param amount The amount the should be distributed to the recipient
+    /// @param merkleProof The merkle proof
     struct Distribution {
         uint256 index;
         address recipientId;
@@ -126,10 +131,11 @@ contract DonationVotingMerkleDistribution is DonationVotingOffchain {
     /// ====================================
 
     /// @notice Distributes funds (tokens) to recipients.
+    /// @param _recipientIds NOT USED
     /// @param _data Data to be decoded
     /// @custom:data (Distribution[] _distributions)
     /// @param _sender The address of the sender
-    function _distribute(address[] memory, bytes memory _data, address _sender)
+    function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender)
         internal
         virtual
         override

contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol

@@ -57,12 +57,14 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native {
     /// ================================
 
     /// @notice Thrown when there is nothing to distribute for the given recipient.
+    /// @param recipientId The recipientId to which distribution was attempted.
     error NOTHING_TO_DISTRIBUTE(address recipientId);
 
     /// @notice Thrown when the timestamps being set or updated don't meet the contracts requirements.
     error INVALID_TIMESTAMPS();
 
     /// @notice Thrown when a the payout for a recipient is attempted to be overwritten.
+    /// @param recipientId The recipientId to which a repeated payout was attempted.
     error PAYOUT_ALREADY_SET(address recipientId);
 
     /// @notice Thrown when the total payout amount is greater than the pool amount.
@@ -76,12 +78,16 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native {
     /// ================================
 
     /// @notice Payout summary struct to hold the payout data
+    /// @param recipientAddress payout address of the recipient
+    /// @param amount payout amount
     struct PayoutSummary {
         address recipientAddress;
         uint256 amount;
     }
 
     /// @notice Struct to hold details of the allocations to claim
+    /// @param recipientId id of the recipient
+    /// @param token token address
     struct Claim {
         address recipientId;
         address token;
@@ -94,8 +100,9 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native {
     /// @notice If true, allocations are directly sent to recipients. Otherwise, they they must be claimed later.
     bool public immutable DIRECT_TRANSFER;
 
-    /// @notice The start and end times for allocations
+    /// @notice The start time for allocations
     uint64 public allocationStartTime;
+    /// @notice The end time for allocations
     uint64 public allocationEndTime;
     /// @notice Cooldown time from allocationEndTime after which the pool manager is allowed to withdraw tokens.
     uint64 public withdrawalCooldown;
@@ -307,8 +314,9 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native {
 
     /// @notice Distributes funds (tokens) to recipients.
     /// @param _recipientIds The IDs of the recipients
+    /// @param _data NOT USED
     /// @param _sender The address of the sender
-    function _distribute(address[] memory _recipientIds, bytes memory, address _sender)
+    function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender)
         internal
         virtual
         override
@@ -332,12 +340,16 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native {
     }
 
     /// @notice Hook called before withdrawing tokens from the pool.
-    function _beforeWithdraw(address, uint256, address) internal virtual override {
+    /// @param _token The address of the token
+    /// @param _amount The amount to withdraw
+    /// @param _recipient The address to withdraw to
+    function _beforeWithdraw(address _token, uint256 _amount, address _recipient) internal virtual override {
         if (block.timestamp <= allocationEndTime + withdrawalCooldown) revert INVALID();
     }
 
-    /// @notice Hook called before increasing the pool amount.
-    function _beforeIncreasePoolAmount(uint256) internal virtual override {
+    /// @notice Hook called after increasing the pool amount.
+    /// @param _amount The amount to increase the pool by
+    function _beforeIncreasePoolAmount(uint256 _amount) internal virtual override {
         if (block.timestamp > allocationEndTime) revert POOL_INACTIVE();
     }