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 5 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
2 changes: 2 additions & 0 deletions .changeset/six-yaks-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
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

---
36 changes: 27 additions & 9 deletions packages/core/solidity/src/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,6 @@ function overrideRawSignatureValidation(c: ContractBuilder, opts: AccountOptions
});
c.addOverride({ name: 'AbstractSigner' }, signerFunctions._rawSignatureValidation);
c.addOverride({ name: 'AccountERC7579' }, signerFunctions._rawSignatureValidation);
c.setFunctionComments(
[
`// IMPORTANT: Make sure Signer${opts.signer} is most derived than AccountERC7579`,
`// in the inheritance chain (i.e. contract ... is AccountERC7579, ..., Signer${opts.signer})`,
'// to ensure the correct order of function resolution.',
'// AccountERC7579 returns false for `_rawSignatureValidation`',
],
signerFunctions._rawSignatureValidation,
);
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to remove this? It's an important consideration for anyone copy pastes the code

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for that. I have reverted it back

// Base override for `_rawSignatureValidation` given MultiSignerERC7913Weighted is MultiSignerERC7913
if (opts.signer === 'MultisigWeighted') {
c.addImportOnly(signers.Multisig);
Expand All @@ -250,6 +241,12 @@ const functions = {
{ name: 'signature', type: 'bytes calldata' },
],
returns: ['bytes4'],
comments: [
'/// @dev Validates a signature for a given hash',
'/// @param hash The hash to validate the signature for',
'/// @param signature The signature to validate',
'/// @return The magic value if the signature is valid, 0 otherwise',
],
},
_validateUserOp: {
kind: 'internal' as const,
Expand All @@ -258,6 +255,12 @@ const functions = {
{ name: 'userOpHash', type: 'bytes32' },
],
returns: ['uint256'],
comments: [
'/// @dev Validates a user operation',
'/// @param userOp The user operation to validate',
'/// @param userOpHash The hash of the user operation',
'/// @return The validation data',
],
},
_erc7821AuthorizedExecutor: {
kind: 'internal' as const,
Expand All @@ -268,25 +271,40 @@ const functions = {
],
returns: ['bool'],
mutability: 'view' as const,
comments: [
'/// @dev Checks if a caller is authorized to execute a specific operation',
'/// @param caller The address of the caller',
'/// @param mode The mode of operation',
'/// @param executionData The data for the execution',
'/// @return True if the caller is authorized, false otherwise',
],
},
addSigners: {
kind: 'public' as const,
args: [{ name: 'signers', type: 'bytes[] memory' }],
comments: ['/// @dev Adds new signers to the account', '/// @param signers Array of signer data to add'],
},
removeSigners: {
kind: 'public' as const,
args: [{ name: 'signers', type: 'bytes[] memory' }],
comments: ['/// @dev Removes signers from the account', '/// @param signers Array of signer data to remove'],
},
setThreshold: {
kind: 'public' as const,
args: [{ name: 'threshold', type: 'uint256' }],
comments: ['/// @dev Sets the threshold for required signatures', '/// @param threshold The new threshold value'],
},
setSignerWeights: {
kind: 'public' as const,
args: [
{ name: 'signers', type: 'bytes[] memory' },
{ name: 'weights', type: 'uint256[] memory' },
],
comments: [
'/// @dev Sets the weights for signers',
'/// @param signers Array of signer data',
'/// @param weights Array of weights corresponding to each signer',
],
},
}),
};
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
26 changes: 26 additions & 0 deletions packages/core/solidity/src/erc1155.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,23 @@ const functions = defineFunctions({
{ name: 'ids', type: 'uint256[] memory' },
{ name: 'values', type: 'uint256[] memory' },
],
comments: [
'/// @dev Updates the balances of `from` and `to` for multiple token IDs',
'/// @param from The address of the sender',
'/// @param to The address of the recipient',
'/// @param ids Array of token IDs to transfer',
'/// @param values Array of amounts to transfer for each token ID',
],
},

setURI: {
kind: 'public' as const,
args: [{ name: 'newuri', type: 'string memory' }],
comments: [
'/// @dev Sets a new URI for all token types',
'/// @param newuri The new URI to set',
'/// @notice This function can only be called by the contract owner',
],
},

mint: {
Expand All @@ -164,6 +176,13 @@ const functions = defineFunctions({
{ name: 'amount', type: 'uint256' },
{ name: 'data', type: 'bytes memory' },
],
comments: [
'/// @dev Creates `amount` tokens of token type `id` and assigns them to `account`',
'/// @param account The address that will receive the tokens',
'/// @param id The token type ID to mint',
'/// @param amount The amount of tokens to mint',
'/// @param data Additional data with no specified format',
],
},

mintBatch: {
Expand All @@ -174,5 +193,12 @@ const functions = defineFunctions({
{ name: 'amounts', type: 'uint256[] memory' },
{ name: 'data', type: 'bytes memory' },
],
comments: [
'/// @dev Creates `amounts` tokens of token types `ids` and assigns them to `to`',
'/// @param to The address that will receive the tokens',
'/// @param ids Array of token type IDs to mint',
'/// @param amounts Array of amounts to mint for each token type',
'/// @param data Additional data with no specified format',
],
},
});
47 changes: 37 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,12 @@ export const functions = defineFunctions({
{ name: 'to', type: 'address' },
{ name: 'value', type: 'uint256' },
],
comments: [
'/// @dev Updates the balances of `from` and `to`',
'/// @param from The address of the sender',
'/// @param to The address of the recipient',
'/// @param value The amount of tokens to transfer',
],
},

_approve: {
Expand All @@ -430,6 +426,13 @@ export const functions = defineFunctions({
{ name: 'value', type: 'uint256' },
{ name: 'emitEvent', type: 'bool' },
],
comments: [
"/// @dev Sets `value` as the allowance of `spender` over the `owner`'s tokens",
'/// @param owner The address of the token owner',
'/// @param spender The address of the spender',
'/// @param value The amount of tokens to approve',
'/// @param emitEvent Whether to emit the Approval event',
],
},

mint: {
Expand All @@ -438,32 +441,56 @@ export const functions = defineFunctions({
{ name: 'to', type: 'address' },
{ name: 'amount', type: 'uint256' },
],
comments: [
'/// @dev Creates `amount` new tokens for `to`',
'/// @param to The address that will receive the minted tokens',
'/// @param amount The amount of tokens to mint',
],
},

pause: {
kind: 'public' as const,
args: [],
comments: [
'/// @dev Pauses all token transfers',
'/// @notice This function can only be called by the contract owner',
],
},

unpause: {
kind: 'public' as const,
args: [],
comments: [
'/// @dev Unpauses all token transfers',
'/// @notice This function can only be called by the contract owner',
],
},

snapshot: {
kind: 'public' as const,
args: [],
comments: ['/// @dev Creates a new snapshot and returns its id', '/// @return The id of the new snapshot'],
},

nonces: {
kind: 'public' as const,
args: [{ name: 'owner', type: 'address' }],
returns: ['uint256'],
mutability: 'view' as const,
comments: [
'/// @dev Returns the current nonce for `owner`',
'/// @param owner The address to get the nonce for',
'/// @return The current nonce of the owner',
],
},

_checkTokenBridge: {
kind: 'internal' as const,
args: [{ name: 'caller', type: 'address' }],
comments: [
'/// @dev Checks if the caller is a valid token bridge',
'/// @param caller The address to check',
'/// @notice This function is used for cross-chain token transfers',
],
},
});
24 changes: 24 additions & 0 deletions packages/core/solidity/src/erc721.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,33 @@ const functions = defineFunctions({
{ name: 'auth', type: 'address' },
],
returns: ['address'],
comments: [
'/// @dev Updates the owner of the token and handles the transfer logic',
'/// @param to The address that will receive the token',
'/// @param tokenId The ID of the token being transferred',
'/// @param auth The address that is authorized to perform the transfer',
'/// @return The address of the previous owner',
],
},

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 `tokenId` token',
'/// @param tokenId The ID of the token to get the URI for',
'/// @return The URI of the token',
],
},

_baseURI: {
kind: 'internal' as const,
args: [],
returns: ['string memory'],
mutability: 'pure' as const,
comments: ['/// @dev Base URI for computing {tokenURI}', '/// @return The base URI string'],
},

_increaseBalance: {
Expand All @@ -243,10 +256,21 @@ const functions = defineFunctions({
{ name: 'account', type: 'address' },
{ name: 'value', type: 'uint128' },
],
comments: [
'/// @dev Increases the balance of `account` by `value`',
'/// @param account The address to increase the balance for',
'/// @param value The amount to increase the balance by',
],
},
});

function getMintFunction(incremental: boolean, uriStorage: boolean): BaseFunction {
/**
* @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
*/
const fn: BaseFunction = {
name: 'safeMint',
kind: 'public' as const,
Expand Down
Loading