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

Outdated MetaTx guide #214

Closed
juanigallo opened this issue Jul 24, 2023 · 5 comments
Closed

Outdated MetaTx guide #214

juanigallo opened this issue Jul 24, 2023 · 5 comments
Labels
guide Guides and tutorials

Comments

@juanigallo
Copy link

The metatx guide currently uses the MinimalForwarder contract as an example. However, this contract was removed in OpenZeppelin/openzeppelin-contracts#4346.

I believe it would be beneficial to update the documentation and utilize ERC2771Forwarder.sol instead.

If you agree with this suggestion, I can create a pull request with the necessary changes

@ernestognw ernestognw transferred this issue from OpenZeppelin/defender-docs Aug 9, 2023
@ernestognw ernestognw transferred this issue from OpenZeppelin/openzeppelin-contracts Aug 9, 2023
@ernestognw
Copy link
Member

ernestognw commented Aug 9, 2023

Hey @juanigallo, the forwarder was updated within contracts 5.0, which hasn't been released yet so this can wait. In any case, I think we should add a warning to the guide mentioning it must use contracts 4.x to prevent any unintentional mistake.

Optionally we can even hide this guide depending on the docs version (we need to check if that's possible).

A PR for this would be very much appreciated! But please understand we may take a bit to review before we decide the strategy for updating guides towards 5.0 but should be around late September for the release candidate.

EDIT: Gosh! sorry for the multiple issue transfers, they didn't go as intended

@ernestognw ernestognw transferred this issue from OpenZeppelin/defender-docs Aug 9, 2023
@frangio frangio transferred this issue from OpenZeppelin/docs.openzeppelin.com Aug 9, 2023
@frangio
Copy link
Contributor

frangio commented Aug 9, 2023

This issue is fine in the defender-docs repo because it relates to Defender documentation content.

I believe it would be beneficial to update the documentation and utilize ERC2771Forwarder.sol instead.

We shouldn't update the guide until ERC2771Forwarder is released in 5.0. In the meantime MinimalForwarder is right.

Adding a warning to mention that MinimalForwarder will be replaced by ERC2771Forwarder in the next OpenZeppelin Contracts release would be fine. No need to hide the guide!

We should prepare the update to publish once 5.0 is released.

@juanigallo
Copy link
Author

Hi @frangio. Is anyone in the team working on this? If not, just let me know and I can create a new guide

@ernestognw
Copy link
Member

Hey @juanigallo, nobody is working on the guide right now but 5.0 is out so feel free to submit a PR.

@ernestognw ernestognw added the guide Guides and tutorials label Oct 26, 2023
@cairoeth
Copy link
Collaborator

cairoeth commented Jan 9, 2024

Updated the guide for Defender 2.0 with #285.

@cairoeth cairoeth closed this as completed Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guide Guides and tutorials
Projects
None yet
Development

No branches or pull requests

4 participants