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

remove attach license terms check from mintLicenseTokens #367

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

jacob-tucker
Copy link
Contributor

This PR removes the following code from mintLicenseTokens:

const isAttachedLicenseTerms =
        await this.licenseRegistryReadOnlyClient.hasIpAttachedLicenseTerms({
          ipId: request.licensorIpId,
          licenseTemplate:
            (request.licenseTemplate &&
              getAddress(request.licenseTemplate, "request.licenseTemplate")) ||
            this.licenseTemplateClient.address,
          licenseTermsId: request.licenseTermsId,
        });
      if (!isAttachedLicenseTerms) {
        throw new Error(
          `License terms id ${request.licenseTermsId} is not attached to the IP with id ${request.licensorIpId}.`,
        );
      }

The reason for this change is because, as the mintLicenseTokens function is described, it should be able to mint private license tokens: "IP owners can mint license tokens for their IPs for arbitrary license terms without attaching the license terms to IP."

The function is currently throwing an error if the terms are not attached, but an IP owner should call this function and be able to mint license tokens even if the terms aren't attached.

I wanted to add a check to see if the IP owner == the sender of the call, but I couldn't find an SDK function to do this, and other functions like attachLicenseTerms don't make this check either.

Copy link
Collaborator

@allenchuang allenchuang left a comment

Choose a reason for hiding this comment

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

will need to address with more complete fix later

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.

3 participants