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

Revert "refactor(permission): enable direct function calls via executeWithSig (#121)" #133

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

sebsadface
Copy link
Member

@sebsadface sebsadface commented Nov 28, 2024

Description

This PR reverts the changes introduced in #121, restoring the use of the permission-setting approach that allows periphery contracts to call protocol core functions on behalf of IP owners. Additionally, it reverts the changes from #128 and #123, which will be reimplemented in the next PR.

Reasons for Reverting the Changes

  • Scalability Concerns with executeWithSig
    After offline discussions, we determined that while executeWithSig offers better security guarantees, it does not scale well with the addition of new core protocol functionalities. Specifically, each protocol interaction requires a user signature, which rapidly increases the stack size of periphery functions and renders them unusable.
    Conversely, the previous permission-setting method does not introduce security risks when proper guardrails are in place. This approach can be further optimized by using setBatchPermission, which reduces the required signatures to a single batch. This simplification greatly improves developer experience and scalability for periphery functions.

  • Reimplementation of Changes from # 128 and # 123
    Changes introduced in feat(licensing): enable multi-license attachment #128 and feat(licensing): support custom license templates and decouple terms registration from attachment #123 were designed around the executeWithSig model, leading to functionality compromises. For instance, the ability to register PIL terms and immediately attach them to IPs was removed because executeWithSig required predicting the new license term ID to generate appropriate signatures.
    By reverting refactor(workflows): replace permission setting with executeWithSig #121, these limitations no longer apply, enabling us to reintroduce these functionalities without compromises in the next PR.

Additional Changes

  • Updated the package to the latest protocol-core main branch to address compilation issues caused by import changes from protocol-core PR #291:
    - import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable-v4/security/ReentrancyGuardUpgradeable.sol";
    + import { ERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
  • Adjusted existing functions to align with the updated core protocol API.

Related Issues

…teWithSig` (storyprotocol#121)"

This reverts commit a6fbf2c.

Revert "feat(licensing): support custom license templates and decouple terms registration from attachment (storyprotocol#123)"

This reverts commit 68f4a4a.

Revert "feat(licensing): enable multi-license attachment (storyprotocol#128)"

This reverts commit 027eed9.
@sebsadface sebsadface merged commit 6016acb into storyprotocol:main Nov 29, 2024
3 checks passed
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