From 08a6b568f0eb977a3b22a9bf7f951d5279d9383e Mon Sep 17 00:00:00 2001 From: Bonnie Date: Tue, 12 Aug 2025 19:29:22 +0800 Subject: [PATCH 1/7] -mRefactor license terms and license config --- .../resources/Group.py | 28 +--- .../resources/IPAsset.py | 123 +++-------------- .../resources/License.py | 63 +++------ src/story_protocol_python_sdk/types/common.py | 7 +- .../utils/constants.py | 6 +- .../utils/license_terms.py | 126 ++++++++++-------- tests/integration/test_integration_group.py | 2 +- tests/unit/fixtures/data.py | 30 +++++ tests/unit/resources/test_license.py | 4 +- tests/unit/utils/test_derivative_data.py | 4 +- 10 files changed, 160 insertions(+), 233 deletions(-) diff --git a/src/story_protocol_python_sdk/resources/Group.py b/src/story_protocol_python_sdk/resources/Group.py index 9776d199..31be4a4c 100644 --- a/src/story_protocol_python_sdk/resources/Group.py +++ b/src/story_protocol_python_sdk/resources/Group.py @@ -547,34 +547,12 @@ def _get_license_data(self, license_data: list) -> list: f'License template address "{license_template}" is invalid.' ) - # Validate licensing config - licensing_config = item.get("licensing_config", {}) - - try: - self.license_terms_util.validate_licensing_config(licensing_config) - except Exception as e: - raise ValueError(f"Licensing config validation failed: {str(e)}") - - # Convert to camelCase for contract interaction - camelcase_config = { - "isSet": licensing_config.get("is_set", True), - "mintingFee": licensing_config.get("minting_fee", 0), - "hookData": licensing_config.get("hook_data", ZERO_ADDRESS), - "licensingHook": licensing_config.get("licensing_hook", ZERO_ADDRESS), - "commercialRevShare": licensing_config.get("commercial_rev_share", 0), - "disabled": licensing_config.get("disabled", False), - "expectMinimumGroupRewardShare": licensing_config.get( - "expect_minimum_group_reward_share", 0 - ), - "expectGroupRewardPool": licensing_config.get( - "expect_group_reward_pool", ZERO_ADDRESS - ), - } - processed_item = { "licenseTemplate": license_template, "licenseTermsId": item["license_terms_id"], - "licensingConfig": camelcase_config, + "licensingConfig": self.license_terms_util.validate_licensing_config( + item.get("licensing_config", {}) + ), } result.append(processed_item) diff --git a/src/story_protocol_python_sdk/resources/IPAsset.py b/src/story_protocol_python_sdk/resources/IPAsset.py index 2aea9ff0..7940fca4 100644 --- a/src/story_protocol_python_sdk/resources/IPAsset.py +++ b/src/story_protocol_python_sdk/resources/IPAsset.py @@ -1,5 +1,6 @@ """Module for handling IP Account operations and transactions.""" +from ens.ens import HexStr from web3 import Web3 from story_protocol_python_sdk.abi.AccessController.AccessController_client import ( @@ -173,7 +174,7 @@ def register( signature_response = self.sign_util.get_permission_signature( ip_id=ip_id, deadline=calculated_deadline, - state=self.web3.to_bytes(hexstr=ZERO_HASH), + state=self.web3.to_bytes(hexstr=HexStr(ZERO_HASH)), permissions=[ { "ipId": ip_id, @@ -366,7 +367,7 @@ def mint_and_register_ip_asset_with_pil_terms( :param commercializer_checker str: Allowed commercializers or zero address for none. :param commercializer_checker_data str: Data for checker contract. - :param commercial_rev_share int: Revenue share percentage. + :param commercial_rev_share int: The commercial revenue share percentage (from 0 to 100%, represented as 100_000_000). :param commercial_rev_ceiling int: Maximum commercial revenue. :param derivatives_allowed bool: Whether derivatives are allowed. :param derivatives_attribution bool: Whether attribution is needed @@ -384,10 +385,10 @@ def mint_and_register_ip_asset_with_pil_terms( :param hook_data str: The data used by the licensing hook. :param licensing_hook str: The licensing hook contract address or address(0) if none. - :param commercial_rev_share int: Commercial revenue share percent. + :param commercial_rev_share int: The commercial revenue share percentage (from 0 to 100%, represented as 100_000_000). :param disabled bool: Whether the license is disabled. :param expect_minimum_group_reward_share int: Minimum group reward - share (0-100%, as 100 * 10^6). + share percentage (from 0 to 100%, represented as 100_000_000). :param expect_group_reward_pool str: Address of the expected group reward pool. :param ip_metadata dict: [Optional] NFT and IP metadata. @@ -405,57 +406,17 @@ def mint_and_register_ip_asset_with_pil_terms( raise ValueError( f"The NFT contract address {spg_nft_contract} is not valid." ) - license_terms = [] for term in terms: - self.license_terms_util.validate_license_terms(term["terms"]) - validated_licensing_config = ( - self.license_terms_util.validate_licensing_config( - term["licensing_config"] - ) - ) - - camelcase_term = { - "transferable": term["terms"]["transferable"], - "royaltyPolicy": term["terms"]["royalty_policy"], - "defaultMintingFee": term["terms"]["default_minting_fee"], - "expiration": term["terms"]["expiration"], - "commercialUse": term["terms"]["commercial_use"], - "commercialAttribution": term["terms"]["commercial_attribution"], - "commercializerChecker": term["terms"]["commercializer_checker"], - "commercializerCheckerData": term["terms"][ - "commercializer_checker_data" - ], - "commercialRevShare": term["terms"]["commercial_rev_share"], - "commercialRevCeiling": term["terms"]["commercial_rev_ceiling"], - "derivativesAllowed": term["terms"]["derivatives_allowed"], - "derivativesAttribution": term["terms"]["derivatives_attribution"], - "derivativesApproval": term["terms"]["derivatives_approval"], - "derivativesReciprocal": term["terms"]["derivatives_reciprocal"], - "derivativeRevCeiling": term["terms"]["derivative_rev_ceiling"], - "currency": term["terms"]["currency"], - "uri": term["terms"]["uri"], - } - - camelcase_config = { - "isSet": validated_licensing_config["is_set"], - "mintingFee": validated_licensing_config["minting_fee"], - "hookData": validated_licensing_config["hook_data"], - "licensingHook": validated_licensing_config["licensing_hook"], - "commercialRevShare": validated_licensing_config[ - "commercial_rev_share" - ], - "disabled": validated_licensing_config["disabled"], - "expectMinimumGroupRewardShare": validated_licensing_config[ - "expect_minimum_group_reward_share" - ], - "expectGroupRewardPool": validated_licensing_config[ - "expect_group_reward_pool" - ], - } - license_terms.append( - {"terms": camelcase_term, "licensingConfig": camelcase_config} + { + "terms": self.license_terms_util.validate_license_terms( + term["terms"] + ), + "licensingConfig": self.license_terms_util.validate_licensing_config( + term["licensing_config"] + ), + } ) metadata = { @@ -632,57 +593,17 @@ def register_ip_and_attach_pil_terms( raise ValueError( f"The NFT with id {token_id} is already registered as IP." ) - license_terms = [] for term in license_terms_data: - self.license_terms_util.validate_license_terms(term["terms"]) - validated_licensing_config = ( - self.license_terms_util.validate_licensing_config( - term["licensing_config"] - ) - ) - - camelcase_term = { - "transferable": term["terms"]["transferable"], - "royaltyPolicy": term["terms"]["royalty_policy"], - "defaultMintingFee": term["terms"]["default_minting_fee"], - "expiration": term["terms"]["expiration"], - "commercialUse": term["terms"]["commercial_use"], - "commercialAttribution": term["terms"]["commercial_attribution"], - "commercializerChecker": term["terms"]["commercializer_checker"], - "commercializerCheckerData": term["terms"][ - "commercializer_checker_data" - ], - "commercialRevShare": term["terms"]["commercial_rev_share"], - "commercialRevCeiling": term["terms"]["commercial_rev_ceiling"], - "derivativesAllowed": term["terms"]["derivatives_allowed"], - "derivativesAttribution": term["terms"]["derivatives_attribution"], - "derivativesApproval": term["terms"]["derivatives_approval"], - "derivativesReciprocal": term["terms"]["derivatives_reciprocal"], - "derivativeRevCeiling": term["terms"]["derivative_rev_ceiling"], - "currency": term["terms"]["currency"], - "uri": term["terms"]["uri"], - } - - camelcase_config = { - "isSet": validated_licensing_config["is_set"], - "mintingFee": validated_licensing_config["minting_fee"], - "hookData": validated_licensing_config["hook_data"], - "licensingHook": validated_licensing_config["licensing_hook"], - "commercialRevShare": validated_licensing_config[ - "commercial_rev_share" - ], - "disabled": validated_licensing_config["disabled"], - "expectMinimumGroupRewardShare": validated_licensing_config[ - "expect_minimum_group_reward_share" - ], - "expectGroupRewardPool": validated_licensing_config[ - "expect_group_reward_pool" - ], - } - license_terms.append( - {"terms": camelcase_term, "licensingConfig": camelcase_config} + { + "terms": self.license_terms_util.validate_license_terms( + term["terms"] + ), + "licensingConfig": self.license_terms_util.validate_licensing_config( + term["licensing_config"] + ), + } ) calculated_deadline = self.sign_util.get_deadline(deadline=deadline) @@ -691,7 +612,7 @@ def register_ip_and_attach_pil_terms( signature_response = self.sign_util.get_permission_signature( ip_id=ip_id, deadline=calculated_deadline, - state=self.web3.to_bytes(hexstr=ZERO_HASH), + state=self.web3.to_bytes(hexstr=HexStr(ZERO_HASH)), permissions=[ { "ipId": ip_id, diff --git a/src/story_protocol_python_sdk/resources/License.py b/src/story_protocol_python_sdk/resources/License.py index 3700c08a..2859654f 100644 --- a/src/story_protocol_python_sdk/resources/License.py +++ b/src/story_protocol_python_sdk/resources/License.py @@ -99,48 +99,27 @@ def register_pil_terms( :return dict: A dictionary with the transaction hash and license terms ID. """ try: - license_terms = { - "transferable": transferable, - "royaltyPolicy": royalty_policy, - "defaultMintingFee": default_minting_fee, - "expiration": expiration, - "commercialUse": commercial_use, - "commercialAttribution": commercial_attribution, - "commercializerChecker": commercializer_checker, - "commercializerCheckerData": commercializer_checker_data, - "commercialRevShare": commercial_rev_share, - "commercialRevCeiling": commercial_rev_ceiling, - "derivativesAllowed": derivatives_allowed, - "derivativesAttribution": derivatives_attribution, - "derivativesApproval": derivatives_approval, - "derivativesReciprocal": derivatives_reciprocal, - "derivativeRevCeiling": derivative_rev_ceiling, - "currency": currency, - "uri": uri, - } - - license_terms_snake = { - "transferable": transferable, - "royalty_policy": royalty_policy, - "default_minting_fee": default_minting_fee, - "expiration": expiration, - "commercial_use": commercial_use, - "commercial_attribution": commercial_attribution, - "commercializer_checker": commercializer_checker, - "commercializer_checker_data": commercializer_checker_data, - "commercial_rev_share": commercial_rev_share, - "commercial_rev_ceiling": commercial_rev_ceiling, - "derivatives_allowed": derivatives_allowed, - "derivatives_attribution": derivatives_attribution, - "derivatives_approval": derivatives_approval, - "derivatives_reciprocal": derivatives_reciprocal, - "derivative_rev_ceiling": derivative_rev_ceiling, - "currency": currency, - "uri": uri, - } - - # Validate the license terms - self.license_terms_util.validate_license_terms(license_terms_snake) + license_terms = self.license_terms_util.validate_license_terms( + { + "transferable": transferable, + "royalty_policy": royalty_policy, + "default_minting_fee": default_minting_fee, + "expiration": expiration, + "commercial_use": commercial_use, + "commercial_attribution": commercial_attribution, + "commercializer_checker": commercializer_checker, + "commercializer_checker_data": commercializer_checker_data, + "commercial_rev_share": commercial_rev_share, + "commercial_rev_ceiling": commercial_rev_ceiling, + "derivatives_allowed": derivatives_allowed, + "derivatives_attribution": derivatives_attribution, + "derivatives_approval": derivatives_approval, + "derivatives_reciprocal": derivatives_reciprocal, + "derivative_rev_ceiling": derivative_rev_ceiling, + "currency": currency, + "uri": uri, + } + ) license_terms_id = self._get_license_terms_id(license_terms) if (license_terms_id is not None) and (license_terms_id != 0): diff --git a/src/story_protocol_python_sdk/types/common.py b/src/story_protocol_python_sdk/types/common.py index 314e2196..d78d9179 100644 --- a/src/story_protocol_python_sdk/types/common.py +++ b/src/story_protocol_python_sdk/types/common.py @@ -2,9 +2,10 @@ class RevShareType(Enum): - COMMERCIAL_REVENUE_SHARE = "commercialRevShare" - MAX_REVENUE_SHARE = "maxRevenueShare" - MAX_ALLOWED_REWARD_SHARE = "maxAllowedRewardShare" + COMMERCIAL_REVENUE_SHARE = "commercial_rev_share" + MAX_REVENUE_SHARE = "max_revenue_share" + MAX_ALLOWED_REWARD_SHARE = "max_allowed_reward_share" + EXPECT_MINIMUM_GROUP_REWARD_SHARE = "expect_minimum_group_reward_share" class AccessPermission(Enum): diff --git a/src/story_protocol_python_sdk/utils/constants.py b/src/story_protocol_python_sdk/utils/constants.py index b30f2769..a0f31171 100644 --- a/src/story_protocol_python_sdk/utils/constants.py +++ b/src/story_protocol_python_sdk/utils/constants.py @@ -1,9 +1,5 @@ -from eth_typing import HexStr - ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" -ZERO_HASH: HexStr = HexStr( - "0x0000000000000000000000000000000000000000000000000000000000000000" -) +ZERO_HASH = "0x0000000000000000000000000000000000000000000000000000000000000000" ZERO_FUNC = "0x00000000" DEFAULT_FUNCTION_SELECTOR = "0x00000000" MAX_ROYALTY_TOKEN = 100000000 diff --git a/src/story_protocol_python_sdk/utils/license_terms.py b/src/story_protocol_python_sdk/utils/license_terms.py index f660f1a9..584efdd4 100644 --- a/src/story_protocol_python_sdk/utils/license_terms.py +++ b/src/story_protocol_python_sdk/utils/license_terms.py @@ -6,10 +6,12 @@ from story_protocol_python_sdk.abi.RoyaltyModule.RoyaltyModule_client import ( RoyaltyModuleClient, ) +from story_protocol_python_sdk.types.common import RevShareType from story_protocol_python_sdk.utils.constants import ( ROYALTY_POLICY_LAP_ADDRESS, ZERO_ADDRESS, ) +from story_protocol_python_sdk.utils.validation import get_revenue_share class LicenseTerms: @@ -119,34 +121,48 @@ def validate_license_terms(self, params): if royalty_policy != ZERO_ADDRESS and currency == ZERO_ADDRESS: raise ValueError("Royalty policy requires currency token.") - params["default_minting_fee"] = int(params.get("default_minting_fee", 0)) - params["expiration"] = int(params.get("expiration", 0)) - params["commercial_rev_ceiling"] = int(params.get("commercial_rev_ceiling", 0)) - params["derivative_rev_ceiling"] = int(params.get("derivative_rev_ceiling", 0)) - - self.verify_commercial_use(params) - self.verify_derivatives(params) - commercial_rev_share = params.get("commercial_rev_share", 0) if commercial_rev_share < 0 or commercial_rev_share > 100: - raise ValueError("CommercialRevShare should be between 0 and 100.") - else: - params["commercial_rev_share"] = int( - (commercial_rev_share / 100) * 100000000 - ) + raise ValueError("commercial_rev_share should be between 0 and 100.") - commercializer_checker_data = params.get( - "commercializer_checker_data", ZERO_ADDRESS + expect_minimum_group_reward_share = params.get( + "expect_minimum_group_reward_share", 0 ) - if isinstance(commercializer_checker_data, str): - params["commercializer_checker_data"] = Web3.to_bytes( - hexstr=HexStr(commercializer_checker_data) + if ( + expect_minimum_group_reward_share < 0 + or expect_minimum_group_reward_share > 100 + ): + raise ValueError( + "Expect minimum group reward share must be between 0 and 100" ) + validated_params = { + "transferable": params.get("transferable"), + "royaltyPolicy": params.get("royalty_policy"), + "defaultMintingFee": int(params.get("default_minting_fee", 0)), + "expiration": int(params.get("expiration", 0)), + "commercialUse": params.get("commercial_use"), + "commercialAttribution": params.get("commercial_attribution"), + "commercializerChecker": params.get("commercializer_checker"), + "commercializerCheckerData": Web3.to_bytes( + hexstr=HexStr(params.get("commercializer_checker_data", ZERO_ADDRESS)) + ), + "commercialRevShare": get_revenue_share( + params.get("commercial_rev_share", 0), + RevShareType.COMMERCIAL_REVENUE_SHARE, + ), + "commercialRevCeiling": int(params.get("commercial_rev_ceiling", 0)), + "derivativesAllowed": params.get("derivatives_allowed"), + "derivativesAttribution": params.get("derivatives_attribution"), + "derivativesApproval": params.get("derivatives_approval"), + "derivativesReciprocal": params.get("derivatives_reciprocal"), + "derivativeRevCeiling": int(params.get("derivative_rev_ceiling", 0)), + "currency": params.get("currency"), + "uri": params.get("uri"), + } - params["expect_minimum_group_reward_share"] = int( - params.get("expect_minimum_group_reward_share", 0) - ) - return params + self.verify_commercial_use(validated_params) + self.verify_derivatives(validated_params) + return validated_params def validate_licensing_config(self, params): if not isinstance(params, dict): @@ -169,14 +185,14 @@ def validate_licensing_config(self, params): raise TypeError(f"{param} must be of type {expected_type.__name__}") default_params = { - "is_set": False, - "minting_fee": 0, - "hook_data": ZERO_ADDRESS, - "licensing_hook": ZERO_ADDRESS, - "commercial_rev_share": 0, + "isSet": False, + "mintingFee": 0, + "hookData": ZERO_ADDRESS, + "licensingHook": ZERO_ADDRESS, + "commercialRevShare": 0, "disabled": False, - "expect_minimum_group_reward_share": 0, - "expect_group_reward_pool": ZERO_ADDRESS, + "expectMinimumGroupRewardShare": 0, + "expectGroupRewardPool": ZERO_ADDRESS, } if not params.get("is_set", False): @@ -190,11 +206,6 @@ def validate_licensing_config(self, params): or params.get("commercial_rev_share", 0) > 100 ): raise ValueError("Commercial revenue share must be between 0 and 100") - else: - params["commercial_rev_share"] = int( - (params["commercial_rev_share"] / 100) * 100000000 - ) - if ( params.get("expect_minimum_group_reward_share", 0) < 0 or params.get("expect_minimum_group_reward_share", 0) > 100 @@ -202,60 +213,71 @@ def validate_licensing_config(self, params): raise ValueError( "Expect minimum group reward share must be between 0 and 100" ) + validated_params = { + "isSet": params.get("is_set", False), + "mintingFee": params.get("minting_fee", 0), + "hookData": Web3.to_bytes(hexstr=HexStr(params["hook_data"])), + "licensingHook": params.get("licensing_hook", ZERO_ADDRESS), + "commercialRevShare": get_revenue_share(params["commercial_rev_share"]), + "disabled": params.get("disabled", False), + "expectMinimumGroupRewardShare": get_revenue_share( + params["expect_minimum_group_reward_share"], + RevShareType.EXPECT_MINIMUM_GROUP_REWARD_SHARE, + ), + "expectGroupRewardPool": params.get( + "expect_group_reward_pool", ZERO_ADDRESS + ), + } - params["hook_data"] = Web3.to_bytes(hexstr=params["hook_data"]) - - default_params.update(params) - - return default_params + return validated_params def verify_commercial_use(self, terms): - if not terms.get("commercial_use", False): - if terms.get("commercial_attribution"): + if not terms.get("commercialUse", False): + if terms.get("commercialAttribution", False): raise ValueError( "Cannot add commercial attribution when commercial use is disabled." ) - if terms.get("commercializer_checker") != ZERO_ADDRESS: + if terms.get("commercializerChecker") != ZERO_ADDRESS: raise ValueError( "Cannot add commercializerChecker when commercial use is disabled." ) - if terms.get("commercial_rev_share", 0) > 0: + if terms.get("commercialRevShare", 0) > 0: raise ValueError( "Cannot add commercial revenue share when commercial use is disabled." ) - if terms.get("commercial_rev_ceiling", 0) > 0: + if terms.get("commercialRevCeiling", 0) > 0: raise ValueError( "Cannot add commercial revenue ceiling when commercial use is disabled." ) - if terms.get("derivative_rev_ceiling", 0) > 0: + if terms.get("derivativeRevCeiling", 0) > 0: raise ValueError( "Cannot add derivative revenue ceiling when commercial use is disabled." ) - if terms.get("royalty_policy") != ZERO_ADDRESS: + if terms.get("royaltyPolicy") != ZERO_ADDRESS: raise ValueError( "Cannot add commercial royalty policy when commercial use is disabled." ) else: - if terms.get("royalty_policy") == ZERO_ADDRESS: + if terms.get("royaltyPolicy") == ZERO_ADDRESS: raise ValueError( "Royalty policy is required when commercial use is enabled." ) def verify_derivatives(self, terms): - if not terms.get("derivatives_allowed", False): - if terms.get("derivatives_attribution"): + if not terms.get("derivativesAllowed", False): + if terms.get("derivativesAttribution", False): raise ValueError( "Cannot add derivative attribution when derivative use is disabled." ) - if terms.get("derivatives_approval"): + if terms.get("derivativesApproval", False): raise ValueError( "Cannot add derivative approval when derivative use is disabled." ) - if terms.get("derivatives_reciprocal"): + if terms.get("derivativesReciprocal", False): raise ValueError( "Cannot add derivative reciprocal when derivative use is disabled." ) - if terms.get("derivative_rev_ceiling", 0) > 0: + if terms.get("derivativeRevCeiling", 0) > 0: raise ValueError( "Cannot add derivative revenue ceiling when derivative use is disabled." ) diff --git a/tests/integration/test_integration_group.py b/tests/integration/test_integration_group.py index a60f790b..fce3a173 100644 --- a/tests/integration/test_integration_group.py +++ b/tests/integration/test_integration_group.py @@ -369,7 +369,7 @@ def setup_royalty_collection(self, story_client, nft_collection): "licensing_hook": ZERO_ADDRESS, "commercial_rev_share": 10, "disabled": False, - "expect_minimum_group_reward_share": 0, + "expect_minimum_group_reward_share": 10, "expect_group_reward_pool": EVEN_SPLIT_GROUP_POOL, }, } diff --git a/tests/unit/fixtures/data.py b/tests/unit/fixtures/data.py index 42203f70..9e2505d9 100644 --- a/tests/unit/fixtures/data.py +++ b/tests/unit/fixtures/data.py @@ -4,3 +4,33 @@ # STATE as bytes32 (32 bytes = 64 hex characters) STATE = b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" IP_ID = "0xFEB4eE75600768635010D80D56a5711268D26DaB" +LICENSE_TERMS = { + "royalty_policy": ADDRESS, + "commercial_rev_share": 19, + "currency": ADDRESS, + "default_minting_fee": 10, + "expiration": 100, + "commercial_use": True, + "commercial_attribution": True, + "commercializer_checker": True, + "commercializer_checker_data": ADDRESS, + "derivatives_allowed": True, + "derivatives_attribution": True, + "derivatives_approval": True, + "derivatives_reciprocal": True, + "derivative_rev_ceiling": 100, + "uri": "https://example.com", + "transferable": True, + "expect_minimum_group_reward_share": 10, +} + +LICENSING_CONFIG = { + "is_set": True, + "minting_fee": 10, + "licensing_hook": ADDRESS, + "hook_data": ADDRESS, + "commercial_rev_share": 10, + "disabled": False, + "expect_minimum_group_reward_share": 10, + "expect_group_reward_pool": ADDRESS, +} diff --git a/tests/unit/resources/test_license.py b/tests/unit/resources/test_license.py index 5ce788e7..f2151f0f 100644 --- a/tests/unit/resources/test_license.py +++ b/tests/unit/resources/test_license.py @@ -187,7 +187,7 @@ def test_register_pil_terms_commercial_rev_share_error_more_than_100( ): with pytest.raises( - ValueError, match="CommercialRevShare should be between 0 and 100." + ValueError, match="commercial_rev_share should be between 0 and 100." ): license_client.register_pil_terms( transferable=False, @@ -225,7 +225,7 @@ def test_register_pil_terms_commercial_rev_share_error_less_than_0( ): with pytest.raises( - ValueError, match="CommercialRevShare should be between 0 and 100." + ValueError, match="commercial_rev_share should be between 0 and 100." ): license_client.register_pil_terms( transferable=False, diff --git a/tests/unit/utils/test_derivative_data.py b/tests/unit/utils/test_derivative_data.py index e90b4695..7014ea61 100644 --- a/tests/unit/utils/test_derivative_data.py +++ b/tests/unit/utils/test_derivative_data.py @@ -309,7 +309,7 @@ def test_validate_max_revenue_share_is_less_than_0( ): with mock_ip_asset_registry_client(), mock_license_registry_client(): with raises( - ValueError, match="The maxRevenueShare must be between 0 and 100." + ValueError, match="max_revenue_share must be between 0 and 100." ): DerivativeData.from_input( web3=mock_web3, @@ -331,7 +331,7 @@ def test_validate_max_revenue_share_is_greater_than_100( ): with mock_is_checksum_address(), mock_ip_asset_registry_client(), mock_license_registry_client(): with raises( - ValueError, match="The maxRevenueShare must be between 0 and 100." + ValueError, match="max_revenue_share must be between 0 and 100." ): DerivativeData.from_input( web3=mock_web3, From eccee640d05d1a8b1b3b50d88fc6f2474d7d29a9 Mon Sep 17 00:00:00 2001 From: Bonnie Date: Tue, 12 Aug 2025 19:30:39 +0800 Subject: [PATCH 2/7] Add unit tests including mint and register_ip_and_attach_pil_terms --- tests/unit/resources/test_ip_asset.py | 174 +++++++++++++++++++++++++- 1 file changed, 172 insertions(+), 2 deletions(-) diff --git a/tests/unit/resources/test_ip_asset.py b/tests/unit/resources/test_ip_asset.py index 45c7e02e..4f9d9744 100644 --- a/tests/unit/resources/test_ip_asset.py +++ b/tests/unit/resources/test_ip_asset.py @@ -5,7 +5,15 @@ from story_protocol_python_sdk.resources.IPAsset import IPAsset from story_protocol_python_sdk.utils.constants import ZERO_HASH from story_protocol_python_sdk.utils.derivative_data import DerivativeDataInput -from tests.unit.fixtures.data import ADDRESS, CHAIN_ID, IP_ID, TX_HASH +from story_protocol_python_sdk.utils.ip_metadata import IPMetadata +from tests.unit.fixtures.data import ( + ADDRESS, + CHAIN_ID, + IP_ID, + LICENSE_TERMS, + LICENSING_CONFIG, + TX_HASH, +) @pytest.fixture(scope="class") @@ -39,7 +47,21 @@ def _mock(is_registered: bool = False): def mock_parse_ip_registered_event(ip_asset): def _mock(): return patch.object( - ip_asset, "_parse_tx_ip_registered_event", return_value={"ip_id": IP_ID} + ip_asset, + "_parse_tx_ip_registered_event", + return_value={"ip_id": IP_ID, "token_id": 1}, + ) + + return _mock + + +@pytest.fixture(scope="class") +def mock_parse_tx_license_terms_attached_event(ip_asset): + def _mock(): + return patch.object( + ip_asset, + "_parse_tx_license_terms_attached_event", + return_value=[1, 2], ) return _mock @@ -172,3 +194,151 @@ def test_success( ) assert result["tx_hash"] == TX_HASH.hex() assert result["ip_id"] == IP_ID + + +class TestMint: + def test_mint_successful(self, ip_asset): + result = ip_asset.mint( + nft_contract=ADDRESS, + to_address=ADDRESS, + metadata_uri="", + metadata_hash=ZERO_HASH, + ) + assert result == f"0x{TX_HASH.hex()}" + + def test_mint_failed_transaction(self, ip_asset): + with patch.object(ip_asset.web3.eth, "send_raw_transaction") as mock_send: + mock_send.side_effect = Exception("Transaction failed") + with pytest.raises(Exception, match="Transaction failed"): + ip_asset.mint( + nft_contract=ADDRESS, + to_address=ADDRESS, + metadata_uri="", + metadata_hash=ZERO_HASH, + allow_duplicates=False, + ) + + +class TestRegisterIpAndAttachPilTerms: + def test_token_id_is_already_registered( + self, ip_asset, mock_get_ip_id, mock_is_registered + ): + with mock_get_ip_id(), mock_is_registered(True): + with pytest.raises( + ValueError, match="The NFT with id 3 is already registered as IP." + ): + ip_asset.register_ip_and_attach_pil_terms( + nft_contract=ADDRESS, + token_id=3, + license_terms_data=[], + ) + + def test_royalty_policy_commercial_rev_share_is_less_than_0( + self, ip_asset, mock_get_ip_id, mock_is_registered + ): + with mock_get_ip_id(), mock_is_registered(): + with pytest.raises( + ValueError, match="commercial_rev_share should be between 0 and 100." + ): + ip_asset.register_ip_and_attach_pil_terms( + nft_contract=ADDRESS, + token_id=3, + license_terms_data=[ + { + "terms": { + **LICENSE_TERMS, + "commercial_rev_share": -1, + }, + } + ], + ) + + def test_transaction_to_be_called_with_correct_parameters( + self, + ip_asset: IPAsset, + mock_get_ip_id, + mock_is_registered, + mock_parse_ip_registered_event, + mock_parse_tx_license_terms_attached_event, + mock_signature_related_methods, + ): + with mock_get_ip_id(), mock_is_registered(), mock_parse_ip_registered_event(), mock_parse_tx_license_terms_attached_event(), mock_signature_related_methods(): + with patch.object( + ip_asset.license_attachment_workflows_client, + "build_registerIpAndAttachPILTerms_transaction", + ) as mock_build_registerIpAndAttachPILTerms_transaction: + + ip_asset.register_ip_and_attach_pil_terms( + nft_contract=ADDRESS, + token_id=3, + license_terms_data=[ + { + "terms": LICENSE_TERMS, + "licensing_config": LICENSING_CONFIG, + } + ], + ) + call_args = mock_build_registerIpAndAttachPILTerms_transaction.call_args[0] + assert call_args[0] == ADDRESS + assert call_args[1] == 3 + assert call_args[2] == IPMetadata.from_input().get_validated_data() + assert call_args[3] == [ + { + "terms": { + "transferable": True, + "royaltyPolicy": "0x1234567890123456789012345678901234567890", + "defaultMintingFee": 10, + "expiration": 100, + "commercialUse": True, + "commercialAttribution": True, + "commercializerChecker": True, + "commercializerCheckerData": b"mock_bytes", + "commercialRevShare": 19000000, + "commercialRevCeiling": 0, + "derivativesAllowed": True, + "derivativesAttribution": True, + "derivativesApproval": True, + "derivativesReciprocal": True, + "derivativeRevCeiling": 100, + "currency": "0x1234567890123456789012345678901234567890", + "uri": "https://example.com", + }, + "licensingConfig": { + "isSet": True, + "mintingFee": 10, + "hookData": b"mock_bytes", + "licensingHook": "0x1234567890123456789012345678901234567890", + "commercialRevShare": 10000000, + "disabled": False, + "expectMinimumGroupRewardShare": 10000000, + "expectGroupRewardPool": "0x1234567890123456789012345678901234567890", + }, + } + ] + + def test_success( + self, + ip_asset: IPAsset, + mock_get_ip_id, + mock_is_registered, + mock_parse_ip_registered_event, + mock_signature_related_methods, + mock_parse_tx_license_terms_attached_event, + ): + with mock_get_ip_id(), mock_is_registered(), mock_parse_ip_registered_event(), mock_parse_tx_license_terms_attached_event(), mock_signature_related_methods(): + result = ip_asset.register_ip_and_attach_pil_terms( + nft_contract=ADDRESS, + token_id=3, + license_terms_data=[ + { + "terms": LICENSE_TERMS, + "licensing_config": LICENSING_CONFIG, + } + ], + ) + assert result == { + "tx_hash": TX_HASH.hex(), + "ip_id": IP_ID, + "license_terms_ids": [1, 2], + "token_id": 1, + } From 8735ef6b69af5172cd81712dc32954e5e02b8fa2 Mon Sep 17 00:00:00 2001 From: Bonnie Date: Wed, 13 Aug 2025 14:52:10 +0800 Subject: [PATCH 3/7] Enhance revenue share handling and documentation across multiple modules - Introduced utility for consistent revenue share calculations. - Updated parameter documentation in , , and classes to clarify revenue share constraints. - Refactored methods to utilize the new revenue share utility, ensuring type safety and clarity. - Added unit tests for revenue share validation and address validation in . --- .../resources/Group.py | 23 ++++--- .../resources/IPAsset.py | 51 ++++++-------- .../resources/License.py | 13 ++-- .../utils/derivative_data.py | 2 +- .../utils/license_terms.py | 17 +---- .../utils/validation.py | 5 +- tests/unit/utils/test_validation.py | 66 +++++++++++++++++++ 7 files changed, 116 insertions(+), 61 deletions(-) create mode 100644 tests/unit/utils/test_validation.py diff --git a/src/story_protocol_python_sdk/resources/Group.py b/src/story_protocol_python_sdk/resources/Group.py index 31be4a4c..2e86bb66 100644 --- a/src/story_protocol_python_sdk/resources/Group.py +++ b/src/story_protocol_python_sdk/resources/Group.py @@ -1,5 +1,6 @@ # src/story_protocol_python_sdk/resources/Group.py +from ens.ens import HexStr from web3 import Web3 from story_protocol_python_sdk.abi.CoreMetadataModule.CoreMetadataModule_client import ( @@ -26,10 +27,12 @@ from story_protocol_python_sdk.abi.PILicenseTemplate.PILicenseTemplate_client import ( PILicenseTemplateClient, ) +from story_protocol_python_sdk.types.common import RevShareType from story_protocol_python_sdk.utils.constants import ZERO_ADDRESS, ZERO_HASH from story_protocol_python_sdk.utils.license_terms import LicenseTerms from story_protocol_python_sdk.utils.sign import Sign from story_protocol_python_sdk.utils.transaction_utils import build_and_send_transaction +from story_protocol_python_sdk.utils.validation import get_revenue_share class Group: @@ -137,7 +140,7 @@ def mint_and_register_ip_and_attach_license_and_add_to_group( :param group_id str: The ID of the group to add the IP to. :param spg_nft_contract str: The address of the SPG NFT contract. :param license_data list: List of license data objects with terms and config. - :param max_allowed_reward_share int: Maximum allowed reward share percentage. + :param max_allowed_reward_share int: Maximum allowed reward share percentage. Must be between 0 and 100 (where 100% represents 100,000,000). :param ip_metadata dict: [Optional] The metadata for the IP. :param recipient str: [Optional] The recipient of the NFT (defaults to caller). :param allow_duplicates bool: [Optional] Whether to allow duplicate IPs. @@ -185,8 +188,8 @@ def mint_and_register_ip_and_attach_license_and_add_to_group( metadata = self._get_ip_metadata(ip_metadata) - max_allowed_reward_share = self.license_terms_util.get_revenue_share( - max_allowed_reward_share + max_allowed_reward_share = get_revenue_share( + max_allowed_reward_share, type=RevShareType.MAX_ALLOWED_REWARD_SHARE ) # Set recipient to caller if not provided @@ -249,7 +252,7 @@ def register_ip_and_attach_license_and_add_to_group( :param nft_contract str: The address of the NFT contract. :param token_id int: The token ID of the NFT. :param license_data list: List of license data objects with terms and config. - :param max_allowed_reward_share int: Maximum allowed reward share percentage. + :param max_allowed_reward_share int: Maximum allowed reward share percentage. Must be between 0 and 100 (where 100% represents 100,000,000). :param ip_metadata dict: [Optional] The metadata for the IP. :param deadline int: [Optional] The deadline for the signature in milliseconds. :param tx_options dict: [Optional] The transaction options. @@ -299,7 +302,7 @@ def register_ip_and_attach_license_and_add_to_group( sig_metadata_and_attach = self.sign_util.get_permission_signature( ip_id=ip_id, deadline=calculated_deadline, - state=self.web3.to_bytes(hexstr=ZERO_HASH), + state=self.web3.to_bytes(hexstr=HexStr(ZERO_HASH)), permissions=[ { "ipId": ip_id, @@ -338,7 +341,9 @@ def register_ip_and_attach_license_and_add_to_group( nft_contract, token_id, group_id, - self.license_terms_util.get_revenue_share(max_allowed_reward_share), + get_revenue_share( + max_allowed_reward_share, type=RevShareType.MAX_ALLOWED_REWARD_SHARE + ), licenses_data, metadata, { @@ -389,7 +394,7 @@ def register_group_and_attach_license_and_add_ips( :param group_pool str: The address of the group pool. :param ip_ids list: List of IP IDs to add to the group. :param license_data dict: License data object with terms and config. - :param max_allowed_reward_share int: Maximum allowed reward share percentage. + :param max_allowed_reward_share int: Maximum allowed reward share percentage. Must be between 0 and 100 (where 100% represents 100,000,000). :param tx_options dict: [Optional] The transaction options. :return dict: A dictionary with the transaction hash and group ID. """ @@ -427,7 +432,9 @@ def register_group_and_attach_license_and_add_ips( self.grouping_workflows_client.build_registerGroupAndAttachLicenseAndAddIps_transaction, group_pool, ip_ids, - self.license_terms_util.get_revenue_share(max_allowed_reward_share), + get_revenue_share( + max_allowed_reward_share, type=RevShareType.MAX_ALLOWED_REWARD_SHARE + ), license_data_processed, tx_options=tx_options, ) diff --git a/src/story_protocol_python_sdk/resources/IPAsset.py b/src/story_protocol_python_sdk/resources/IPAsset.py index 7940fca4..7bc160dd 100644 --- a/src/story_protocol_python_sdk/resources/IPAsset.py +++ b/src/story_protocol_python_sdk/resources/IPAsset.py @@ -247,7 +247,7 @@ def register_derivative( if set to 0 then no limit :param max_rts int: The maximum number of royalty tokens that can be distributed (max: 100,000,000) - :param max_revenue_share int: The maximum revenue share percentage allowed (0-100,000,000) + :param max_revenue_share int: The maximum revenue share percentage allowed. Must be between 0 and 100 (where 100% represents 100,000,000). Default is 100. :param license_template str: [Optional] The license template address :param tx_options dict: [Optional] Transaction options :return dict: A dictionary with the transaction hash @@ -257,31 +257,24 @@ def register_derivative( raise ValueError( f"The child IP with id {child_ip_id} is not registered." ) - - derivative_data = self._validate_derivative_data( - { - "childIpId": child_ip_id, - "parentIpIds": parent_ip_ids, - "licenseTermsIds": license_terms_ids, - "maxMintingFee": max_minting_fee, - "maxRts": max_rts, - "maxRevenueShare": max_revenue_share, - "licenseTemplate": license_template, - } - ) + derivative_data = DerivativeData.from_input( + web3=self.web3, + input_data=DerivativeDataInput( + parent_ip_ids=parent_ip_ids, + license_terms_ids=license_terms_ids, + max_minting_fee=max_minting_fee, + max_rts=max_rts, + max_revenue_share=max_revenue_share, + license_template=license_template, + ), + ).get_validated_data() response = build_and_send_transaction( self.web3, self.account, self.licensing_module_client.build_registerDerivative_transaction, - derivative_data["childIpId"], - derivative_data["parentIpIds"], - derivative_data["licenseTermsIds"], - derivative_data["licenseTemplate"], - derivative_data["royaltyContext"], - derivative_data["maxMintingFee"], - derivative_data["maxRts"], - derivative_data["maxRevenueShare"], + child_ip_id, + **derivative_data, tx_options=tx_options, ) @@ -367,7 +360,7 @@ def mint_and_register_ip_asset_with_pil_terms( :param commercializer_checker str: Allowed commercializers or zero address for none. :param commercializer_checker_data str: Data for checker contract. - :param commercial_rev_share int: The commercial revenue share percentage (from 0 to 100%, represented as 100_000_000). + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param commercial_rev_ceiling int: Maximum commercial revenue. :param derivatives_allowed bool: Whether derivatives are allowed. :param derivatives_attribution bool: Whether attribution is needed @@ -385,12 +378,10 @@ def mint_and_register_ip_asset_with_pil_terms( :param hook_data str: The data used by the licensing hook. :param licensing_hook str: The licensing hook contract address or address(0) if none. - :param commercial_rev_share int: The commercial revenue share percentage (from 0 to 100%, represented as 100_000_000). + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param disabled bool: Whether the license is disabled. - :param expect_minimum_group_reward_share int: Minimum group reward - share percentage (from 0 to 100%, represented as 100_000_000). - :param expect_group_reward_pool str: Address of the expected group - reward pool. + :param expect_minimum_group_reward_share int: Minimum group reward share percentage. Must be between 0 and 100 (where 100% represents 100,000,000). + :param expect_group_reward_pool str: Address of the expected group reward pool. :param ip_metadata dict: [Optional] NFT and IP metadata. :param ip_metadata_uri str: [Optional] IP metadata URI. :param ip_metadata_hash str: [Optional] IP metadata hash. @@ -560,7 +551,7 @@ def register_ip_and_attach_pil_terms( :param commercial_attribution bool: Whether attribution is required when reproducing the work commercially or not. :param commercializer_checker str: Commercializers that are allowed to commercially exploit the work. :param commercializer_checker_data str: The data to be passed to the commercializer checker contract. - :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param commercial_rev_ceiling int: The maximum revenue that can be generated from the commercial use of the work. :param derivatives_allowed bool: Indicates whether the licensee can create derivatives of his work or not. :param derivatives_attribution bool: Indicates whether attribution is required for derivatives of the work or not. @@ -574,9 +565,9 @@ def register_ip_and_attach_pil_terms( :param minting_fee int: The minting fee to be paid when minting license tokens. :param licensing_hook str: The hook contract address for the licensing module. :param hook_data str: The data to be used by the licensing hook. - :param commercial_rev_share int: The commercial revenue share percentage. + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param disabled bool: Whether the licensing is disabled or not. - :param expect_minimum_group_reward_share int: The minimum percentage of the group's reward share. + :param expect_minimum_group_reward_share int: The minimum percentage of the group's reward share. Must be between 0 and 100 (where 100% represents 100,000,000). :param expect_group_reward_pool str: The address of the expected group reward pool. :param ip_metadata dict: [Optional] The metadata for the newly registered IP. :param ip_metadata_uri str: [Optional] The URI of the metadata for the IP. diff --git a/src/story_protocol_python_sdk/resources/License.py b/src/story_protocol_python_sdk/resources/License.py index 2859654f..aa5e4fcd 100644 --- a/src/story_protocol_python_sdk/resources/License.py +++ b/src/story_protocol_python_sdk/resources/License.py @@ -18,9 +18,11 @@ from story_protocol_python_sdk.abi.PILicenseTemplate.PILicenseTemplate_client import ( PILicenseTemplateClient, ) +from story_protocol_python_sdk.types.common import RevShareType from story_protocol_python_sdk.utils.constants import ZERO_ADDRESS from story_protocol_python_sdk.utils.license_terms import LicenseTerms from story_protocol_python_sdk.utils.transaction_utils import build_and_send_transaction +from story_protocol_python_sdk.utils.validation import get_revenue_share class License: @@ -86,7 +88,7 @@ def register_pil_terms( :param commercial_attribution bool: Whether attribution is required when reproducing the work commercially or not. :param commercializer_checker str: Commercializers that are allowed to commercially exploit the work. If zero address, then no restrictions is enforced. :param commercializer_checker_data str: The data to be passed to the commercializer checker contract. - :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param commercial_rev_ceiling int: The maximum revenue that can be generated from the commercial use of the work. :param derivatives_allowed bool: Indicates whether the licensee can create derivatives of his work or not. :param derivatives_attribution bool: Indicates whether attribution is required for derivatives of the work or not. @@ -237,7 +239,7 @@ def register_commercial_remix_pil( :param default_minting_fee int: The fee to be paid when minting a license. :param currency str: The ERC20 token to be used to pay the minting fee. - :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. + :param commercial_rev_share int: Percentage of revenue that must be shared with the licensor. Must be between 0 and 100 (where 100% represents 100,000,000). :param royalty_policy str: The address of the royalty policy contract. :param tx_options dict: [Optional] The transaction options. :return dict: A dictionary with the transaction hash and the license terms ID. @@ -368,7 +370,7 @@ def mint_license_tokens( :param amount int: The amount of license tokens to mint. :param receiver str: The address of the receiver. :param max_minting_fee int: [Optional] The maximum minting fee that the caller is willing to pay. If set to 0 then no limit. Defaults to 0. - :param max_revenue_share int: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100,000,000 (where 100,000,000 represents 100%). Defaults to 0. + :param max_revenue_share int: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100,000,000 (where 100,000,000 represents 100%). Defaults to 100. :param tx_options dict: [Optional] The transaction options. :return dict: A dictionary with the transaction hash and the license token IDs. """ @@ -410,7 +412,10 @@ def mint_license_tokens( receiver, ZERO_ADDRESS, # Zero address for royalty context max_minting_fee, - self.license_terms_util.get_revenue_share(max_revenue_share), + get_revenue_share( + 100 if max_revenue_share is None else max_revenue_share, + RevShareType.MAX_REVENUE_SHARE, + ), tx_options=tx_options, ) diff --git a/src/story_protocol_python_sdk/utils/derivative_data.py b/src/story_protocol_python_sdk/utils/derivative_data.py index 47aeacc1..7927e785 100644 --- a/src/story_protocol_python_sdk/utils/derivative_data.py +++ b/src/story_protocol_python_sdk/utils/derivative_data.py @@ -30,7 +30,7 @@ class DerivativeDataInput: license_terms_ids: List of license terms IDs corresponding to each parent IP. max_minting_fee: [Optional] The maximum minting fee that the caller is willing to pay. if set to 0 then no limit. (default: 0). max_rts: [Optional] The maximum number of royalty tokens that can be distributed to the external royalty policies. (max: 100,000,000) (default: 100,000,000). - max_revenue_share: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100 (where 100% represents 100_000_000) (default: 100). + max_revenue_share: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100 (where 100% represents 100,000,000) (default: 100). license_template: [Optional] The address of the license template. Defaults to [License Template](https://docs.story.foundation/docs/programmable-ip-license) address if not provided """ diff --git a/src/story_protocol_python_sdk/utils/license_terms.py b/src/story_protocol_python_sdk/utils/license_terms.py index 584efdd4..7b190b31 100644 --- a/src/story_protocol_python_sdk/utils/license_terms.py +++ b/src/story_protocol_python_sdk/utils/license_terms.py @@ -92,9 +92,7 @@ def get_license_term_by_type(self, type, term=None): "currency": term["currency"], "commercialUse": True, "commercialAttribution": True, - "commercialRevShare": int( - (term["commercialRevShare"] / 100) * 100000000 - ), + "commercialRevShare": get_revenue_share(term["commercialRevShare"]), "derivativesReciprocal": True, "royaltyPolicy": term["royaltyPolicyAddress"], } @@ -125,16 +123,6 @@ def validate_license_terms(self, params): if commercial_rev_share < 0 or commercial_rev_share > 100: raise ValueError("commercial_rev_share should be between 0 and 100.") - expect_minimum_group_reward_share = params.get( - "expect_minimum_group_reward_share", 0 - ) - if ( - expect_minimum_group_reward_share < 0 - or expect_minimum_group_reward_share > 100 - ): - raise ValueError( - "Expect minimum group reward share must be between 0 and 100" - ) validated_params = { "transferable": params.get("transferable"), "royaltyPolicy": params.get("royalty_policy"), @@ -147,8 +135,7 @@ def validate_license_terms(self, params): hexstr=HexStr(params.get("commercializer_checker_data", ZERO_ADDRESS)) ), "commercialRevShare": get_revenue_share( - params.get("commercial_rev_share", 0), - RevShareType.COMMERCIAL_REVENUE_SHARE, + params.get("commercial_rev_share", 0) ), "commercialRevCeiling": int(params.get("commercial_rev_ceiling", 0)), "derivativesAllowed": params.get("derivatives_allowed"), diff --git a/src/story_protocol_python_sdk/utils/validation.py b/src/story_protocol_python_sdk/utils/validation.py index 5d0155b2..85a603d8 100644 --- a/src/story_protocol_python_sdk/utils/validation.py +++ b/src/story_protocol_python_sdk/utils/validation.py @@ -1,7 +1,6 @@ from web3 import Web3 from story_protocol_python_sdk.types.common import RevShareType -from story_protocol_python_sdk.utils.constants import MAX_ROYALTY_TOKEN def validate_address(address: str) -> str: @@ -25,10 +24,10 @@ def get_revenue_share( Convert revenue share percentage to token amount. :param revShare int: Revenue share percentage between 0-100 - :param type RevShareType: Type of revenue share + :param type RevShareType: Type of revenue share, default is commercial revenue share :return int: Revenue share token amount """ if revShare < 0 or revShare > 100: raise ValueError(f"The {type.value} must be between 0 and 100.") - return (revShare * MAX_ROYALTY_TOKEN) // 100 + return revShare * 10**6 diff --git a/tests/unit/utils/test_validation.py b/tests/unit/utils/test_validation.py new file mode 100644 index 00000000..1e458cd8 --- /dev/null +++ b/tests/unit/utils/test_validation.py @@ -0,0 +1,66 @@ +import pytest + +from story_protocol_python_sdk.types.common import RevShareType +from story_protocol_python_sdk.utils.validation import ( + get_revenue_share, + validate_address, +) + + +class TestValidateAddress: + def test_valid_address(self): + address = "0x1234567890123456789012345678901234567890" + assert validate_address(address) == address + + def test_invalid_address(self): + with pytest.raises(ValueError, match="Invalid address: invalid_address."): + validate_address("invalid_address") + + +class TestGetRevenueShare: + def test_valid_revenue_share_of_100(self): + assert get_revenue_share(100) == 100 * 10**6 + + def test_valid_revenue_share_of_0(self): + assert get_revenue_share(0, RevShareType.COMMERCIAL_REVENUE_SHARE) == 0 + + def test_valid_revenue_share_of_50(self): + assert ( + get_revenue_share(50, RevShareType.MAX_ALLOWED_REWARD_SHARE) == 50 * 10**6 + ) + + def test_valid_revenue_share_with_type_of_100(self): + assert get_revenue_share(100) == 100 * 10**6 + + def test_revenue_share_less_than_0_with_commercial_revenue_share(self): + with pytest.raises( + ValueError, match="The commercial_rev_share must be between 0 and 100." + ): + get_revenue_share(-1) + + def test_revenue_share_greater_than_100_with_max_allowed_reward_share(self): + with pytest.raises( + ValueError, match="The max_allowed_reward_share must be between 0 and 100." + ): + get_revenue_share(101, RevShareType.MAX_ALLOWED_REWARD_SHARE) + + def test_revenue_share_greater_than_100_with_commercial_revenue_share(self): + with pytest.raises( + ValueError, match="The commercial_rev_share must be between 0 and 100." + ): + get_revenue_share(101, RevShareType.COMMERCIAL_REVENUE_SHARE) + + def test_revenue_share_less_than_0_with_max_revenue_share(self): + with pytest.raises( + ValueError, match="The max_revenue_share must be between 0 and 100." + ): + get_revenue_share(-1, RevShareType.MAX_REVENUE_SHARE) + + def test_revenue_share_greater_than_100_with_expect_minimum_group_reward_share( + self, + ): + with pytest.raises( + ValueError, + match="The expect_minimum_group_reward_share must be between 0 and 100.", + ): + get_revenue_share(101, RevShareType.EXPECT_MINIMUM_GROUP_REWARD_SHARE) From ac1d06fd27dfd3c3a3d7c6c16e9df6c5fbaf34d5 Mon Sep 17 00:00:00 2001 From: Bonnie Date: Thu, 14 Aug 2025 10:23:46 +0800 Subject: [PATCH 4/7] Refactor register_derivative method to unpack derivative_data parameters explicitly. --- src/story_protocol_python_sdk/resources/IPAsset.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/story_protocol_python_sdk/resources/IPAsset.py b/src/story_protocol_python_sdk/resources/IPAsset.py index 7bc160dd..13a2e28d 100644 --- a/src/story_protocol_python_sdk/resources/IPAsset.py +++ b/src/story_protocol_python_sdk/resources/IPAsset.py @@ -274,7 +274,13 @@ def register_derivative( self.account, self.licensing_module_client.build_registerDerivative_transaction, child_ip_id, - **derivative_data, + derivative_data["parentIpIds"], + derivative_data["licenseTermsIds"], + derivative_data["licenseTemplate"], + derivative_data["royaltyContext"], + derivative_data["maxMintingFee"], + derivative_data["maxRts"], + derivative_data["maxRevenueShare"], tx_options=tx_options, ) From 9dd5aa768644e7d01499916c27f0a823f2d3b38c Mon Sep 17 00:00:00 2001 From: Bonnie Date: Thu, 14 Aug 2025 10:50:50 +0800 Subject: [PATCH 5/7] fix: set default max_revenue_share to 100 and update related tests --- .../resources/License.py | 4 +- tests/unit/resources/test_license.py | 92 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/story_protocol_python_sdk/resources/License.py b/src/story_protocol_python_sdk/resources/License.py index aa5e4fcd..83ca0dcb 100644 --- a/src/story_protocol_python_sdk/resources/License.py +++ b/src/story_protocol_python_sdk/resources/License.py @@ -358,7 +358,7 @@ def mint_license_tokens( amount: int, receiver: str, max_minting_fee: int = 0, - max_revenue_share: int = 0, + max_revenue_share: int = 100, tx_options: dict | None = None, ) -> dict: """ @@ -413,7 +413,7 @@ def mint_license_tokens( ZERO_ADDRESS, # Zero address for royalty context max_minting_fee, get_revenue_share( - 100 if max_revenue_share is None else max_revenue_share, + max_revenue_share, RevShareType.MAX_REVENUE_SHARE, ), tx_options=tx_options, diff --git a/tests/unit/resources/test_license.py b/tests/unit/resources/test_license.py index f2151f0f..9237855b 100644 --- a/tests/unit/resources/test_license.py +++ b/tests/unit/resources/test_license.py @@ -1,10 +1,12 @@ from unittest.mock import MagicMock, patch import pytest +from _pytest.fixtures import fixture from eth_utils import is_address, to_checksum_address from web3 import Web3 from story_protocol_python_sdk.resources.License import License +from tests.unit.fixtures.data import ADDRESS, CHAIN_ID, IP_ID ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" VALID_ADDRESS = "0x1daAE3197Bc469Cb97B917aa460a12dD95c6627c" @@ -832,3 +834,93 @@ def test_set_licensing_config_zero_address_with_rev_share(self, license_client): license_template=ZERO_ADDRESS, licensing_config=config, ) + + +######################################################################################## +##TODO: Need to refactor the previous test case + + +@fixture +def license(mock_web3, mock_account): + return License(web3=mock_web3, account=mock_account, chain_id=CHAIN_ID) + + +@fixture +def patch_is_registered(license): + def _patch(is_registered=True): + return patch.object( + license.ip_asset_registry_client, "isRegistered", return_value=is_registered + ) + + return _patch + + +@fixture +def patch_exists(license): + def _patch(exists=True): + return patch.object( + license.license_template_client, "exists", return_value=exists + ) + + return _patch + + +@fixture +def patch_has_ip_attached_license_terms(license): + def _patch(has_ip_attached_license_terms=True): + return patch.object( + license.license_registry_client, + "hasIpAttachedLicenseTerms", + return_value=has_ip_attached_license_terms, + ) + + return _patch + + +class TestMintLicenseTokens: + def test_defaults_to_100_max_revenue_share_when_not_provided( + self, + license: License, + patch_is_registered, + patch_exists, + patch_has_ip_attached_license_terms, + ): + with patch_is_registered(), patch_exists(), patch_has_ip_attached_license_terms(): + with patch.object( + license.licensing_module_client, + "build_mintLicenseTokens_transaction", + ) as mock_build_mintLicenseTokens_transaction: + + license.mint_license_tokens( + licensor_ip_id=IP_ID, + license_template=ADDRESS, + license_terms_id=1, + amount=1, + receiver=ZERO_ADDRESS, + ) + max_revenue_share = mock_build_mintLicenseTokens_transaction.call_args[0][7] + assert max_revenue_share == 100 * 10**6 + + def test_max_revenue_share_is_10_when_provided( + self, + license: License, + patch_is_registered, + patch_exists, + patch_has_ip_attached_license_terms, + ): + with patch_is_registered(), patch_exists(), patch_has_ip_attached_license_terms(): + with patch.object( + license.licensing_module_client, + "build_mintLicenseTokens_transaction", + ) as mock_build_mintLicenseTokens_transaction: + + license.mint_license_tokens( + licensor_ip_id=IP_ID, + license_template=ADDRESS, + license_terms_id=1, + amount=1, + receiver=ZERO_ADDRESS, + max_revenue_share=10, + ) + max_revenue_share = mock_build_mintLicenseTokens_transaction.call_args[0][7] + assert max_revenue_share == 10 * 10**6 From 0dc0a087360fff33345c800ae4adeddeb2c554e6 Mon Sep 17 00:00:00 2001 From: Bonnie Date: Thu, 14 Aug 2025 11:12:42 +0800 Subject: [PATCH 6/7] refactor: update default values for max_rts and max_revenue_share in IPAsset class, enhance documentation, and add unit tests for register_derivative method --- .../resources/IPAsset.py | 18 +++-- tests/unit/resources/test_ip_asset.py | 65 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/story_protocol_python_sdk/resources/IPAsset.py b/src/story_protocol_python_sdk/resources/IPAsset.py index 13a2e28d..a9cb4465 100644 --- a/src/story_protocol_python_sdk/resources/IPAsset.py +++ b/src/story_protocol_python_sdk/resources/IPAsset.py @@ -35,7 +35,11 @@ ) from story_protocol_python_sdk.abi.SPGNFTImpl.SPGNFTImpl_client import SPGNFTImplClient from story_protocol_python_sdk.types.common import AccessPermission -from story_protocol_python_sdk.utils.constants import ZERO_ADDRESS, ZERO_HASH +from story_protocol_python_sdk.utils.constants import ( + MAX_ROYALTY_TOKEN, + ZERO_ADDRESS, + ZERO_HASH, +) from story_protocol_python_sdk.utils.derivative_data import ( DerivativeData, DerivativeDataInput, @@ -228,8 +232,8 @@ def register_derivative( parent_ip_ids: list, license_terms_ids: list, max_minting_fee: int = 0, - max_rts: int = 0, - max_revenue_share: int = 0, + max_rts: int = MAX_ROYALTY_TOKEN, + max_revenue_share: int = 100, license_template: str | None = None, tx_options: dict | None = None, ) -> dict: @@ -244,11 +248,11 @@ def register_derivative( :param parent_ip_ids list: The parent IP IDs :param license_terms_ids list: The IDs of the license terms that the parent IP supports :param max_minting_fee int: The maximum minting fee that the caller is willing to pay. - if set to 0 then no limit + if set to 0 then no limit. (default: 0) :param max_rts int: The maximum number of royalty tokens that can be distributed - (max: 100,000,000) - :param max_revenue_share int: The maximum revenue share percentage allowed. Must be between 0 and 100 (where 100% represents 100,000,000). Default is 100. - :param license_template str: [Optional] The license template address + (max: 100,000,000) (default: 100,000,000) + :param max_revenue_share int: The maximum revenue share percentage allowed. Must be between 0 and 100 (where 100% represents 100,000,000). (default: 100) + :param license_template str: [Optional] The license template address. Defaults to [License Template](https://docs.story.foundation/docs/programmable-ip-license) address if not provided. :param tx_options dict: [Optional] Transaction options :return dict: A dictionary with the transaction hash """ diff --git a/tests/unit/resources/test_ip_asset.py b/tests/unit/resources/test_ip_asset.py index 4f9d9744..fcbc0fd9 100644 --- a/tests/unit/resources/test_ip_asset.py +++ b/tests/unit/resources/test_ip_asset.py @@ -342,3 +342,68 @@ def test_success( "license_terms_ids": [1, 2], "token_id": 1, } + + +class TestRegisterDerivative: + def test_default_value_when_not_provided( + self, + ip_asset: IPAsset, + mock_get_ip_id, + mock_is_registered, + mock_parse_ip_registered_event, + mock_license_registry_client, + ): + with mock_get_ip_id(), mock_is_registered( + True + ), mock_parse_ip_registered_event(), mock_license_registry_client(): + with patch.object( + ip_asset.licensing_module_client, + "build_registerDerivative_transaction", + ) as mock_build_registerDerivative_transaction: + + ip_asset.register_derivative( + child_ip_id=IP_ID, + parent_ip_ids=[IP_ID, IP_ID], + license_terms_ids=[1, 2], + ) + call_args = mock_build_registerDerivative_transaction.call_args[0] + print(call_args) + assert ( + call_args[3] == "0x1234567890123456789012345678901234567890" + ) # license_template + assert ( + call_args[4] == "0x0000000000000000000000000000000000000000" + ) # royalty_context + assert call_args[5] == 0 # max_minting_fee + assert call_args[6] == 100000000 # max_rts + assert call_args[7] == 100 * 10**6 # max_revenue_share + + def test_call_value_when_provided( + self, + ip_asset: IPAsset, + mock_get_ip_id, + mock_is_registered, + mock_parse_ip_registered_event, + mock_license_registry_client, + ): + with mock_get_ip_id(), mock_is_registered( + True + ), mock_parse_ip_registered_event(), mock_license_registry_client(): + with patch.object( + ip_asset.licensing_module_client, + "build_registerDerivative_transaction", + ) as mock_build_registerDerivative_transaction: + ip_asset.register_derivative( + child_ip_id=IP_ID, + parent_ip_ids=[IP_ID, IP_ID], + license_terms_ids=[1, 2], + max_revenue_share=10, + max_minting_fee=10, + max_rts=100, + license_template=ADDRESS, + ) + call_args = mock_build_registerDerivative_transaction.call_args[0] + assert call_args[7] == 10 * 10**6 # max_revenue_share + assert call_args[5] == 10 # max_minting_fee + assert call_args[6] == 100 # max_rts + assert call_args[3] == ADDRESS # license_template From 4501f06ce1734eb0882f48fffcb2bf234f37bcf9 Mon Sep 17 00:00:00 2001 From: Bonnie Date: Thu, 14 Aug 2025 11:17:15 +0800 Subject: [PATCH 7/7] refactor: improve documentation for max_minting_fee and max_revenue_share parameters in License class, update unit tests for default values --- .../resources/License.py | 4 ++-- tests/unit/resources/test_license.py | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/story_protocol_python_sdk/resources/License.py b/src/story_protocol_python_sdk/resources/License.py index 83ca0dcb..0f553aca 100644 --- a/src/story_protocol_python_sdk/resources/License.py +++ b/src/story_protocol_python_sdk/resources/License.py @@ -369,8 +369,8 @@ def mint_license_tokens( :param license_terms_id int: The ID of the license terms within the license template. :param amount int: The amount of license tokens to mint. :param receiver str: The address of the receiver. - :param max_minting_fee int: [Optional] The maximum minting fee that the caller is willing to pay. If set to 0 then no limit. Defaults to 0. - :param max_revenue_share int: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100,000,000 (where 100,000,000 represents 100%). Defaults to 100. + :param max_minting_fee int: [Optional] The maximum minting fee that the caller is willing to pay. If set to 0 then no limit. (default: 0) + :param max_revenue_share int: [Optional] The maximum revenue share percentage allowed for minting the License Tokens. Must be between 0 and 100,000,000 (where 100,000,000 represents 100%). (default: 100) :param tx_options dict: [Optional] The transaction options. :return dict: A dictionary with the transaction hash and the license token IDs. """ diff --git a/tests/unit/resources/test_license.py b/tests/unit/resources/test_license.py index 9237855b..8934200b 100644 --- a/tests/unit/resources/test_license.py +++ b/tests/unit/resources/test_license.py @@ -878,7 +878,7 @@ def _patch(has_ip_attached_license_terms=True): class TestMintLicenseTokens: - def test_defaults_to_100_max_revenue_share_when_not_provided( + def test_default_value_when_not_provided( self, license: License, patch_is_registered, @@ -898,10 +898,11 @@ def test_defaults_to_100_max_revenue_share_when_not_provided( amount=1, receiver=ZERO_ADDRESS, ) - max_revenue_share = mock_build_mintLicenseTokens_transaction.call_args[0][7] - assert max_revenue_share == 100 * 10**6 + call_args = mock_build_mintLicenseTokens_transaction.call_args[0] + assert call_args[6] == 0 # max_minting_fee + assert call_args[7] == 100 * 10**6 # max_revenue_share - def test_max_revenue_share_is_10_when_provided( + def test_call_value_when_provided( self, license: License, patch_is_registered, @@ -921,6 +922,8 @@ def test_max_revenue_share_is_10_when_provided( amount=1, receiver=ZERO_ADDRESS, max_revenue_share=10, + max_minting_fee=10, ) - max_revenue_share = mock_build_mintLicenseTokens_transaction.call_args[0][7] - assert max_revenue_share == 10 * 10**6 + call_args = mock_build_mintLicenseTokens_transaction.call_args[0] + assert call_args[6] == 10 # max_minting_fee + assert call_args[7] == 10 * 10**6 # max_revenue_share