Skip to content

Feat/59 add natspec descriptions #549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
38fe538
Add detailed comments to functions across multiple Solidity files for…
mrscottyrose May 13, 2025
ff4dd4f
Remove unnecessary comments from function definitions in Solidity fil…
mrscottyrose May 13, 2025
ad24f84
Refactor function definitions in Solidity files to enhance readabilit…
mrscottyrose May 13, 2025
50636ed
Delete .changeset/salty-clowns-shake.md
mrscottyrose May 14, 2025
688b242
Remove deprecated .changeset file and clean up Solidity function comm…
mrscottyrose May 14, 2025
9d40424
update NatSpec comments to be more insightful
mrscottyrose May 16, 2025
2a54021
revert back the args
mrscottyrose May 16, 2025
eca0d50
refactor: update NatSpec comments to be more insightful
mrscottyrose May 20, 2025
ce61955
fix: update mutability from 'pure' to 'view' for ERC721 functions and…
mrscottyrose May 21, 2025
8e9e569
refactor: replace internal _burn function with public mint function i…
mrscottyrose May 21, 2025
cc564f9
refactor: replace internal _burn function with public mint function i…
mrscottyrose May 22, 2025
d0b743b
Update packages/core/solidity/src/erc721.ts
mrscottyrose May 22, 2025
35212b8
Merge branch 'fix/59-add-natspec-descriptions' of https://github.com/…
mrscottyrose May 22, 2025
8cf4a81
refactor: update mutability types, and remove unused internal mint fu…
mrscottyrose May 22, 2025
37dc690
refactor: update mutability types, and remove unused internal mint fu…
mrscottyrose May 26, 2025
946288b
clean duplicate comments and add the removed arguments
mrscottyrose May 26, 2025
5c44e50
Merge branch 'fix/59-add-natspec-descriptions' of https://github.com/…
mrscottyrose May 26, 2025
23b1a74
refactor: remove outdated NatSpec comments for _checkTokenBridge func…
mrscottyrose May 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/six-yaks-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing detailed change-set description

"@openzeppelin/contracts-wizard": patch
---
Add NatSpec descriptions to generated contract functions.
29 changes: 28 additions & 1 deletion packages/core/solidity/src/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -268,25 +275,45 @@ 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,
args: [
{ 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.',
],
},
}),
};
5 changes: 5 additions & 0 deletions packages/core/solidity/src/common-functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
};
1 change: 1 addition & 0 deletions packages/core/solidity/src/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface BaseFunction {
returns?: string[];
kind: FunctionKind;
mutability?: FunctionMutability;
comments?: string[];
}

export interface ContractFunction extends BaseFunction {
Expand Down
12 changes: 12 additions & 0 deletions packages/core/solidity/src/erc1155.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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: {
Expand All @@ -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.',
],
},
});
38 changes: 28 additions & 10 deletions packages/core/solidity/src/erc20.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,16 +400,6 @@ function addSuperchainERC20(c: ContractBuilder) {
functions._checkTokenBridge,
'pure',
);
c.setFunctionComments(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment text is still relevant for this scenario. It would be fine for it to overwrite the generic function comment for _checkTokenBridge

[
'/**',
' * @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({
Expand All @@ -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: {
Expand All @@ -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: {
Expand All @@ -438,32 +434,54 @@ 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: {
kind: 'public' as const,
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.',
],
},
});
18 changes: 18 additions & 0 deletions packages/core/solidity/src/erc721.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,29 @@ 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: {
kind: 'public' as const,
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: {
kind: 'internal' as const,
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: {
Expand All @@ -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',
Expand Down
Loading
Loading