diff --git a/.changeset/six-yaks-accept.md b/.changeset/six-yaks-accept.md new file mode 100644 index 000000000..b2aef1ac8 --- /dev/null +++ b/.changeset/six-yaks-accept.md @@ -0,0 +1,4 @@ +--- +"@openzeppelin/contracts-wizard": patch +--- +Add NatSpec descriptions to generated contract functions. diff --git a/packages/core/solidity/src/account.ts b/packages/core/solidity/src/account.ts index 4001a19f1..c8b65a76d 100644 --- a/packages/core/solidity/src/account.ts +++ b/packages/core/solidity/src/account.ts @@ -233,7 +233,6 @@ function overrideRawSignatureValidation(c: ContractBuilder, opts: AccountOptions ], signerFunctions._rawSignatureValidation, ); - // Base override for `_rawSignatureValidation` given MultiSignerERC7913Weighted is MultiSignerERC7913 if (opts.signer === 'MultisigWeighted') { c.addImportOnly(signers.Multisig); } @@ -250,6 +249,10 @@ const functions = { { name: 'signature', type: 'bytes calldata' }, ], returns: ['bytes4'], + comments: [ + '/// @dev Validates a signature for a given hash.', + '/// Must return the correct magic value for valid signatures.', + ], }, _validateUserOp: { kind: 'internal' as const, @@ -258,6 +261,10 @@ const functions = { { name: 'userOpHash', type: 'bytes32' }, ], returns: ['uint256'], + comments: [ + '/// @dev Validates a user operation during account abstraction.', + '/// Consider implementing additional validation logic for security.', + ], }, _erc7821AuthorizedExecutor: { kind: 'internal' as const, @@ -268,18 +275,34 @@ const functions = { ], returns: ['bool'], mutability: 'view' as const, + comments: [ + '/// @dev Checks if a caller is authorized to execute a specific operation.', + '/// Implements the ERC-7821 standard for authorized execution.', + ], }, addSigners: { kind: 'public' as const, args: [{ name: 'signers', type: 'bytes[] memory' }], + comments: [ + '/// @dev Adds new signers to the account.', + '/// Consider implementing access control to restrict who can add signers.', + ], }, removeSigners: { kind: 'public' as const, args: [{ name: 'signers', type: 'bytes[] memory' }], + comments: [ + '/// @dev Removes signers from the account.', + '/// Consider implementing access control to restrict who can remove signers.', + ], }, setThreshold: { kind: 'public' as const, args: [{ name: 'threshold', type: 'uint256' }], + comments: [ + '/// @dev Sets the threshold for required signatures.', + '/// Consider implementing validation to ensure the threshold is reasonable.', + ], }, setSignerWeights: { kind: 'public' as const, @@ -287,6 +310,10 @@ const functions = { { name: 'signers', type: 'bytes[] memory' }, { name: 'weights', type: 'uint256[] memory' }, ], + comments: [ + '/// @dev Sets the weights for signers.', + '/// Consider implementing validation to ensure weights are reasonable.', + ], }, }), }; diff --git a/packages/core/solidity/src/common-functions.ts b/packages/core/solidity/src/common-functions.ts index cf311c898..f6d9c2a4e 100644 --- a/packages/core/solidity/src/common-functions.ts +++ b/packages/core/solidity/src/common-functions.ts @@ -6,4 +6,9 @@ export const supportsInterface: BaseFunction = { args: [{ name: 'interfaceId', type: 'bytes4' }], returns: ['bool'], mutability: 'view', + comments: [ + '/// @dev Checks if the contract implements a specific interface', + '/// @param interfaceId The interface identifier to check', + '/// @return True if the contract implements the interface, false otherwise', + ], }; diff --git a/packages/core/solidity/src/contract.ts b/packages/core/solidity/src/contract.ts index 6ed276124..e754bc1d0 100644 --- a/packages/core/solidity/src/contract.ts +++ b/packages/core/solidity/src/contract.ts @@ -41,6 +41,7 @@ export interface BaseFunction { returns?: string[]; kind: FunctionKind; mutability?: FunctionMutability; + comments?: string[]; } export interface ContractFunction extends BaseFunction { diff --git a/packages/core/solidity/src/erc1155.ts b/packages/core/solidity/src/erc1155.ts index 9bb93ded1..2873fae9b 100644 --- a/packages/core/solidity/src/erc1155.ts +++ b/packages/core/solidity/src/erc1155.ts @@ -149,11 +149,17 @@ const functions = defineFunctions({ { name: 'ids', type: 'uint256[] memory' }, { name: 'values', type: 'uint256[] memory' }, ], + comments: [ + '/// @dev Hook that is called during any token transfer. This includes minting and burning. Override this function to add custom logic for token transfers.', + ], }, setURI: { kind: 'public' as const, args: [{ name: 'newuri', type: 'string memory' }], + comments: [ + '/// @dev Sets a new URI for all token types. This function can only be called by the contract owner and should be used to update the metadata location.', + ], }, mint: { @@ -164,6 +170,9 @@ const functions = defineFunctions({ { name: 'amount', type: 'uint256' }, { name: 'data', type: 'bytes memory' }, ], + comments: [ + '/// @dev Creates new tokens and assigns them to the specified account. This function should be called with proper access control to restrict who can mint tokens.', + ], }, mintBatch: { @@ -174,5 +183,8 @@ const functions = defineFunctions({ { name: 'amounts', type: 'uint256[] memory' }, { name: 'data', type: 'bytes memory' }, ], + comments: [ + '/// @dev Creates multiple token types in a single transaction. This is more gas efficient than calling mint multiple times.', + ], }, }); diff --git a/packages/core/solidity/src/erc20.ts b/packages/core/solidity/src/erc20.ts index e9abf7525..21f651b64 100644 --- a/packages/core/solidity/src/erc20.ts +++ b/packages/core/solidity/src/erc20.ts @@ -400,16 +400,6 @@ function addSuperchainERC20(c: ContractBuilder) { functions._checkTokenBridge, 'pure', ); - c.setFunctionComments( - [ - '/**', - ' * @dev Checks if the caller is the predeployed SuperchainTokenBridge. Reverts otherwise.', - ' *', - ' * IMPORTANT: The predeployed SuperchainTokenBridge is only available on chains in the Superchain.', - ' */', - ], - functions._checkTokenBridge, - ); } export const functions = defineFunctions({ @@ -420,6 +410,9 @@ export const functions = defineFunctions({ { name: 'to', type: 'address' }, { name: 'value', type: 'uint256' }, ], + comments: [ + '/// @dev Hook that is called during any token transfer. This includes minting and burning. Override this function to add custom logic for token transfers.', + ], }, _approve: { @@ -430,6 +423,9 @@ export const functions = defineFunctions({ { name: 'value', type: 'uint256' }, { name: 'emitEvent', type: 'bool' }, ], + comments: [ + '/// @dev Sets the allowance for a spender to spend tokens on behalf of an owner. This is an internal function that should be called from a public function with proper access control.', + ], }, mint: { @@ -438,21 +434,36 @@ export const functions = defineFunctions({ { name: 'to', type: 'address' }, { name: 'amount', type: 'uint256' }, ], + comments: [ + '/// @dev Creates new tokens and assigns them to the specified account. This function should be called with proper access control to restrict who can mint tokens.', + ], }, pause: { kind: 'public' as const, args: [], + comments: [ + '/// @dev Pauses all token transfers.', + '/// Consider implementing access control to restrict who can pause.', + ], }, unpause: { kind: 'public' as const, args: [], + comments: [ + '/// @dev Unpauses all token transfers.', + '/// Consider implementing access control to restrict who can unpause.', + ], }, snapshot: { kind: 'public' as const, args: [], + comments: [ + '/// @dev Creates a new snapshot and returns its id.', + '/// Consider implementing access control to restrict who can create snapshots.', + ], }, nonces: { @@ -460,10 +471,17 @@ export const functions = defineFunctions({ args: [{ name: 'owner', type: 'address' }], returns: ['uint256'], mutability: 'view' as const, + comments: [ + '/// @dev Returns the current nonce for an owner.', + '/// Used for permit signatures to prevent replay attacks.', + ], }, _checkTokenBridge: { kind: 'internal' as const, args: [{ name: 'caller', type: 'address' }], + comments: [ + '/// @dev Validates if the caller is an authorized token bridge. This is used for cross-chain token transfers to ensure only trusted bridges can move tokens.', + ], }, }); diff --git a/packages/core/solidity/src/erc721.ts b/packages/core/solidity/src/erc721.ts index 2b945123f..2be75a0d0 100644 --- a/packages/core/solidity/src/erc721.ts +++ b/packages/core/solidity/src/erc721.ts @@ -221,6 +221,9 @@ const functions = defineFunctions({ { name: 'auth', type: 'address' }, ], returns: ['address'], + comments: [ + '/// @dev Hook that is called during any token transfer. This includes minting and burning. Override this function to add custom logic for token transfers.', + ], }, tokenURI: { @@ -228,6 +231,9 @@ const functions = defineFunctions({ args: [{ name: 'tokenId', type: 'uint256' }], returns: ['string memory'], mutability: 'view' as const, + comments: [ + '/// @dev Returns the Uniform Resource Identifier (URI) for the specified token. This is used to store metadata for each token.', + ], }, _baseURI: { @@ -235,6 +241,9 @@ const functions = defineFunctions({ args: [], returns: ['string memory'], mutability: 'pure' as const, + comments: [ + '/// @dev Base URI for computing tokenURI. If set, the resulting URI for each token will be the concatenation of the baseURI and the tokenId.', + ], }, _increaseBalance: { @@ -243,9 +252,18 @@ const functions = defineFunctions({ { name: 'account', type: 'address' }, { name: 'value', type: 'uint128' }, ], + comments: [ + '/// @dev Increases the balance of an account. This is used internally to track token ownership and should not be called directly.', + ], }, }); +/** + * @dev Creates a mint function configuration based on the specified options + * @param incremental Whether to use incremental token IDs + * @param uriStorage Whether to include URI storage functionality + * @return The configured mint function + */ function getMintFunction(incremental: boolean, uriStorage: boolean): BaseFunction { const fn: BaseFunction = { name: 'safeMint', diff --git a/packages/core/solidity/src/governor.ts b/packages/core/solidity/src/governor.ts index a8d64f010..b8b3db655 100644 --- a/packages/core/solidity/src/governor.ts +++ b/packages/core/solidity/src/governor.ts @@ -403,37 +403,64 @@ const functions = defineFunctions({ returns: ['uint256'], kind: 'public', mutability: 'pure', + comments: ['/// @dev Delay between proposal creation and voting start.', '/// Used to prevent flash loan attacks.'], }, + votingPeriod: { args: [], returns: ['uint256'], kind: 'public', mutability: 'pure', + comments: [ + '/// @dev Duration of the voting period.', + '/// Consider the time needed for users to make informed decisions.', + ], }, + proposalThreshold: { args: [], returns: ['uint256'], kind: 'public', mutability: 'pure', + comments: [ + '/// @dev Minimum number of votes required to create a proposal.', + '/// Used to prevent spam proposals.', + ], }, + proposalNeedsQueuing: { args: [{ name: 'proposalId', type: 'uint256' }], returns: ['bool'], kind: 'public', mutability: 'view', + comments: [ + '/// @dev Checks if a proposal needs to be queued before execution.', + '/// Used for proposals that modify critical parameters.', + ], }, + quorum: { args: [{ name: 'blockNumber', type: 'uint256' }], returns: ['uint256'], kind: 'public', mutability: 'view', + comments: [ + '/// @dev Returns the quorum required at a specific timepoint.', + '/// Consider implementing a dynamic quorum mechanism.', + ], }, + quorumDenominator: { args: [], returns: ['uint256'], kind: 'public', mutability: 'view', + comments: [ + '/// @dev Denominator used to express quorum as a fraction of total token supply.', + '/// Used to calculate the minimum number of votes required.', + ], }, + propose: { args: [ { name: 'targets', type: 'address[] memory' }, @@ -443,7 +470,9 @@ const functions = defineFunctions({ ], returns: ['uint256'], kind: 'public', + comments: ['/// @dev Creates a new proposal.', '/// Consider implementing validation for proposal parameters.'], }, + _propose: { args: [ { name: 'targets', type: 'address[] memory' }, @@ -454,7 +483,12 @@ const functions = defineFunctions({ ], returns: ['uint256'], kind: 'internal', + comments: [ + '/// @dev Internal function to create a proposal.', + '/// Consider implementing additional validation logic.', + ], }, + _queueOperations: { args: [ { name: 'proposalId', type: 'uint256' }, @@ -463,9 +497,11 @@ const functions = defineFunctions({ { name: 'calldatas', type: 'bytes[] memory' }, { name: 'descriptionHash', type: 'bytes32' }, ], - kind: 'internal', returns: ['uint48'], + kind: 'internal', + comments: ['/// @dev Queues a proposal for execution.', '/// Consider implementing a timelock mechanism.'], }, + _executeOperations: { args: [ { name: 'proposalId', type: 'uint256' }, @@ -475,7 +511,9 @@ const functions = defineFunctions({ { name: 'descriptionHash', type: 'bytes32' }, ], kind: 'internal', + comments: ['/// @dev Executes a queued proposal.', '/// Consider implementing additional security checks.'], }, + _cancel: { args: [ { name: 'targets', type: 'address[] memory' }, @@ -485,17 +523,25 @@ const functions = defineFunctions({ ], returns: ['uint256'], kind: 'internal', + comments: ['/// @dev Cancels a proposal.', '/// Consider implementing access control for cancellation.'], }, + state: { args: [{ name: 'proposalId', type: 'uint256' }], returns: ['ProposalState'], kind: 'public', mutability: 'view', + comments: ['/// @dev Returns the current state of a proposal.', '/// Used to track the proposal lifecycle.'], }, + _executor: { args: [], returns: ['address'], kind: 'internal', mutability: 'view', + comments: [ + '/// @dev Returns the address that can execute a proposal.', + '/// Consider implementing a custom executor mechanism.', + ], }, }); diff --git a/packages/core/solidity/src/set-clock-mode.ts b/packages/core/solidity/src/set-clock-mode.ts index 189a952cc..3f7b3e1c9 100644 --- a/packages/core/solidity/src/set-clock-mode.ts +++ b/packages/core/solidity/src/set-clock-mode.ts @@ -11,6 +11,10 @@ const functions = defineFunctions({ args: [], returns: ['uint48'], mutability: 'view' as const, + comments: [ + '/// @dev Returns the current timestamp as a uint48.', + '/// Used for time-based operations in the contract.', + ], }, CLOCK_MODE: { @@ -25,7 +29,6 @@ export function setClockMode(c: ContractBuilder, parent: ReferencedContract, vot if (votes === 'timestamp') { c.addOverride(parent, functions.clock); c.setFunctionBody(['return uint48(block.timestamp);'], functions.clock); - c.setFunctionComments(['// solhint-disable-next-line func-name-mixedcase'], functions.CLOCK_MODE); c.addOverride(parent, functions.CLOCK_MODE); c.setFunctionBody(['return "mode=timestamp";'], functions.CLOCK_MODE); diff --git a/packages/core/solidity/src/set-upgradeable.ts b/packages/core/solidity/src/set-upgradeable.ts index ff6502805..574adcc10 100644 --- a/packages/core/solidity/src/set-upgradeable.ts +++ b/packages/core/solidity/src/set-upgradeable.ts @@ -44,7 +44,10 @@ export function setUpgradeable(c: ContractBuilder, upgradeable: Upgradeable, acc const functions = defineFunctions({ _authorizeUpgrade: { - args: [{ name: 'newImplementation', type: 'address' }], kind: 'internal', + args: [{ name: 'newImplementation', type: 'address' }], + comments: [ + '/// @dev Authorizes the upgrade to a new implementation. This function should be overridden to implement access control for upgrades.', + ], }, }); diff --git a/packages/core/solidity/src/signer.ts b/packages/core/solidity/src/signer.ts index b5441e6f0..a137a359c 100644 --- a/packages/core/solidity/src/signer.ts +++ b/packages/core/solidity/src/signer.ts @@ -81,6 +81,9 @@ export const signerFunctions = { initializeECDSA: { kind: 'public' as const, args: [{ name: 'signer', type: 'address' }], + comments: [ + '/// @dev Initializes the contract with an ECDSA signer. This function should be called during contract deployment to set up the signing mechanism.', + ], }, initializeP256: { kind: 'public' as const, @@ -88,6 +91,9 @@ export const signerFunctions = { { name: 'qx', type: 'bytes32' }, { name: 'qy', type: 'bytes32' }, ], + comments: [ + '/// @dev Initializes the contract with a P256 signer. This function should be called during contract deployment to set up the signing mechanism.', + ], }, initializeRSA: { kind: 'public' as const, @@ -95,6 +101,9 @@ export const signerFunctions = { { name: 'e', type: 'bytes memory' }, { name: 'n', type: 'bytes memory' }, ], + comments: [ + '/// @dev Initializes the contract with an RSA signer. This function should be called during contract deployment to set up the signing mechanism.', + ], }, initializeMultisig: { kind: 'public' as const, @@ -102,6 +111,9 @@ export const signerFunctions = { { name: 'signers', type: 'bytes[] memory' }, { name: 'threshold', type: 'uint256' }, ], + comments: [ + '/// @dev Initializes the contract with multiple signers. This function should be called during contract deployment to set up the multisig mechanism.', + ], }, initializeMultisigWeighted: { kind: 'public' as const, @@ -110,6 +122,9 @@ export const signerFunctions = { { name: 'weights', type: 'uint256[] memory' }, { name: 'threshold', type: 'uint256' }, ], + comments: [ + '/// @dev Initializes the contract with weighted signers. This function should be called during contract deployment to set up the weighted multisig mechanism.', + ], }, _rawSignatureValidation: { kind: 'internal' as const, diff --git a/packages/core/solidity/src/stablecoin.ts b/packages/core/solidity/src/stablecoin.ts index 8d1134989..c5a1b4da1 100644 --- a/packages/core/solidity/src/stablecoin.ts +++ b/packages/core/solidity/src/stablecoin.ts @@ -138,26 +138,31 @@ const functions = { args: [{ name: 'user', type: 'address' }], returns: ['bool'], mutability: 'view' as const, + comments: ['/// @dev Determines if an address has custodian privileges. Custodians can perform special operations on behalf of users.'], }, allowUser: { kind: 'public' as const, args: [{ name: 'user', type: 'address' }], + comments: ['/// @dev Grants permission for a user to interact with the contract. This is part of the allowlist mechanism for controlled access.'], }, disallowUser: { kind: 'public' as const, args: [{ name: 'user', type: 'address' }], + comments: ['/// @dev Revokes a user\'s permission to interact with the contract. This is part of the allowlist mechanism for controlled access.'], }, blockUser: { kind: 'public' as const, args: [{ name: 'user', type: 'address' }], + comments: ['/// @dev Blocks a user from interacting with the contract. This is part of the blocklist mechanism for controlled access.'], }, unblockUser: { kind: 'public' as const, args: [{ name: 'user', type: 'address' }], + comments: ['/// @dev Removes a user from the blocklist, restoring their ability to interact with the contract.'], }, }), };