Skip to content
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

Disable transpilation of contracts with @custom:stateless in peerProject mode #132

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

Amxx
Copy link
Contributor

@Amxx Amxx commented Sep 25, 2023

marking a contract as /// @custom:stateless will disable the transpilation of a contract when operating in peer project mode. These contracts will be treated as libraries or interfaces.

Note that marking a contract as stateless cannot be undone without introducing a breaking change.

Here are some contracts that I believe should be marked as stateless in @openzeppelin/contracts:

  • Context
  • ERC165
  • Multicall
  • ERC721Holder
  • ERC1155Holder

@frangio
Copy link
Contributor

frangio commented Sep 25, 2023

Not sure about the peer part. What other modes do you have in mind?

If we go forward with that format, we should validate the arguments and reject anything that is not peer.

That said, I'm not sure I like the annotation name. Is it something we would want to have in the vanilla codebase, or only added in the upgradeable patch? I'm thinking that it will show up in vanilla verified source code otherwise, which is weird.

Another option we could consider is @custom:stateless.

@frangio frangio mentioned this pull request Sep 25, 2023
@Amxx
Copy link
Contributor Author

Amxx commented Sep 26, 2023

Not sure about the peer part. What other modes do you have in mind?

I'm was thinking that in the future another mode could be used to exclude a contract from the transpilation when -q is not enabled. Something like -x but in the source code.

@Amxx
Copy link
Contributor Author

Amxx commented Sep 26, 2023

I like @custom:stateless.

@Amxx Amxx changed the title Add a natspec comment to disable transpilation when in peerProject mode Disable transpilation of contracts marked as @custom:stateless when in peerProject mode. Sep 26, 2023
@@ -14,7 +14,7 @@ export function* extractNatspec(node: {
typeof node.documentation === 'string' ? node.documentation : node.documentation?.text ?? '';

for (const { groups } of execall(
/^\s*(?:@(?<title>\w+)(?::(?<tag>[a-z][a-z-]*))? )?(?<args>(?:(?!^\s@\w+)[^])*)/m,
/^\s*(?:@(?<title>\w+)(?::(?<tag>[a-z][a-z-]*))?)?(?: (?<args>(?:(?!^\s@\w+)[^])*))?/m,
Copy link
Contributor Author

@Amxx Amxx Sep 26, 2023

Choose a reason for hiding this comment

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

FYI, the issue here is that the space in the tag block prevents parsing title+tag without a space. It is not an issue when there are args, like in @custom:storage-size 50, but it showed up with @custom:stateless not having args.

@Amxx
Copy link
Contributor Author

Amxx commented Sep 26, 2023

Note: the last refactor removes many of the change from 0.3.28. In particular, exclude is once again a boolean (no more soft exclude). Soft exclusion is implemented by removing file from the Transform object after construction.

@frangio frangio changed the title Disable transpilation of contracts marked as @custom:stateless when in peerProject mode. Disable transpilation of contracts with @custom:stateless in peerProject mode Sep 26, 2023
@frangio frangio merged commit eb97aef into OpenZeppelin:master Sep 27, 2023
2 checks passed
@Amxx Amxx deleted the feature/need-not-transpile branch September 27, 2023 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants