Skip to content

Conversation

@ericglau
Copy link
Member

@ericglau ericglau commented Feb 12, 2024

Remove safe allowance option and functions as per OpenZeppelin/cairo-contracts#881

@ericglau ericglau changed the title Remove non-standard increase_allowance and decrease_allowance from ERC20 Cairo: Remove non-standard increase_allowance and decrease_allowance from ERC20 Feb 12, 2024
@netlify
Copy link

netlify bot commented Feb 12, 2024

Deploy Preview for openzeppelin-contracts-wizard ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/openzeppelin-contracts-wizard/deploys/65ca481ba108d42598512f9f
😎 Deploy Preview https://deploy-preview-324--openzeppelin-contracts-wizard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ericglau ericglau force-pushed the removeSafeAllowance branch from 188c69c to 1d782ae Compare February 12, 2024 17:02
@ericglau ericglau force-pushed the removeSafeAllowance branch from 1d782ae to 9b4413f Compare February 12, 2024 17:07
Copy link
Contributor

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

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

Looking good, Eric! I think there's a few other remnants of SafeAllowance to get rid of:

1, 2
3

@ericglau
Copy link
Member Author

@andrew-fleming Thanks for noticing! Updated.

@andrew-fleming
Copy link
Contributor

Ah another thing: it's now deprecated to add the external attribute on generated trait impls:

    #[generate_trait]
    #[external(v0)]
    impl ExternalImpl of ExternalTrait {
        foo() {}
    }

Instead of using the external attribute, we can now use abi(per_item) and add the external attr to each function within the trait e.g.

    #[generate_trait]
    #[abi(per_item)]
    impl ExternalImpl of ExternalTrait {
        #[external(v0)]
        fn foo() {}
    }

Here's an example in the lib

The alternative is to use loose functions and not define them within a trait. Having the external trait doesn't really matter AFAICT. We need to make a decision on this for the wizard @martriay @ericnordelo

@ericglau
Copy link
Member Author

ericglau commented Feb 12, 2024

@andrew-fleming I made the change suggested in your comment above, and also updated to use abi(embed_v0) for interface impls.

Pending further decision on whether we should use loose functions instead, we could address that in a separate issue if needed.

Copy link
Contributor

@andrew-fleming andrew-fleming left a comment

Choose a reason for hiding this comment

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

LGTM!

@ericglau ericglau merged commit 779751b into OpenZeppelin:master Feb 12, 2024
@ericglau ericglau deleted the removeSafeAllowance branch February 12, 2024 21:32
@github-actions github-actions bot mentioned this pull request Jun 20, 2025
@github-actions github-actions bot mentioned this pull request Aug 20, 2025
This was referenced Sep 9, 2025
@github-actions github-actions bot mentioned this pull request Sep 16, 2025
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.

2 participants