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.

Sign up for GitHub

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

feat(royalty): remove currencyTokens parameter and extract internally in batch claim functions #75

Merged
merged 3 commits into from
Sep 24, 2024
+147 −33
Merged

feat(royalty): remove currencyTokens parameter and extract internally in batch claim functions #75

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dda4afd
feat(royalty): rm `currencyTokens` from para list
sebsadface Sep 23, 2024
322a560
test(royalty): update tests & add more asserts
sebsadface Sep 23, 2024
45ec772
improve error handling in RoyaltyWorkflows
sebsadface Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions contracts/interfaces/workflows/IRoyaltyWorkflows.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,20 @@ interface IRoyaltyWorkflows {
/// and claims revenue on that snapshot for each specified currency token.
/// @param ancestorIpId The address of the ancestor IP.
/// @param claimer The address of the claimer of the revenue tokens (must be a royalty token holder).
/// @param currencyTokens The addresses of the currency (revenue) tokens to claim (each address must be unique).
/// @param royaltyClaimDetails The details of the royalty claim from child IPs,
/// see {IRoyaltyWorkflows-RoyaltyClaimDetails}.
/// @return snapshotId The ID of the snapshot taken.
/// @return amountsClaimed The amount of revenue claimed for each currency token.
function transferToVaultAndSnapshotAndClaimByTokenBatch(
address ancestorIpId,
address claimer,
address[] calldata currencyTokens,
RoyaltyClaimDetails[] calldata royaltyClaimDetails
) external returns (uint256 snapshotId, uint256[] memory amountsClaimed);

/// @notice Transfers royalties to the ancestor IP's royalty vault, takes a snapshot, claims revenue for each
/// specified currency token both on the new snapshot and on each specified unclaimed snapshots.
/// @param ancestorIpId The address of the ancestor IP.
/// @param claimer The address of the claimer of the revenue tokens (must be a royalty token holder).
/// @param currencyTokens The addresses of the currency (revenue) tokens to claim (each address must be unique).
/// @param unclaimedSnapshotIds The IDs of unclaimed snapshots to include in the claim.
/// @param royaltyClaimDetails The details of the royalty claim from child IPs,
/// see {IRoyaltyWorkflows-RoyaltyClaimDetails}.
Expand All @@ -45,7 +42,6 @@ interface IRoyaltyWorkflows {
function transferToVaultAndSnapshotAndClaimBySnapshotBatch(
address ancestorIpId,
address claimer,
address[] calldata currencyTokens,
uint256[] calldata unclaimedSnapshotIds,
RoyaltyClaimDetails[] calldata royaltyClaimDetails
) external returns (uint256 snapshotId, uint256[] memory amountsClaimed);
Expand Down
48 changes: 41 additions & 7 deletions contracts/workflows/RoyaltyWorkflows.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,13 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
/// and claims revenue on that snapshot for each specified currency token.
/// @param ancestorIpId The address of the ancestor IP.
/// @param claimer The address of the claimer of the revenue tokens (must be a royalty token holder).
/// @param currencyTokens The addresses of the currency (revenue) tokens to claim (each address must be unique).
/// @param royaltyClaimDetails The details of the royalty claim from child IPs,
/// see {IRoyaltyWorkflows-RoyaltyClaimDetails}.
/// @return snapshotId The ID of the snapshot taken.
/// @return amountsClaimed The amount of revenue claimed for each currency token.
function transferToVaultAndSnapshotAndClaimByTokenBatch(
address ancestorIpId,
address claimer,
address[] calldata currencyTokens,
RoyaltyClaimDetails[] calldata royaltyClaimDetails
) external returns (uint256 snapshotId, uint256[] memory amountsClaimed) {
// Transfers to ancestor's vault an amount of revenue tokens claimable via the given royalty policy
Expand All @@ -75,7 +73,7 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
// Claims revenue for each specified currency token from the latest snapshot
amountsClaimed = ancestorIpRoyaltyVault.claimRevenueOnBehalfByTokenBatch({
snapshotId: snapshotId,
tokenList: currencyTokens,
tokenList: _getCurrencyTokenList(royaltyClaimDetails),
claimer: claimer
});
}
Expand All @@ -84,7 +82,6 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
/// specified currency token both on the new snapshot and on each specified unclaimed snapshots.
/// @param ancestorIpId The address of the ancestor IP.
/// @param claimer The address of the claimer of the revenue tokens (must be a royalty token holder).
/// @param currencyTokens The addresses of the currency (revenue) tokens to claim (each address must be unique).
/// @param unclaimedSnapshotIds The IDs of unclaimed snapshots to include in the claim.
/// @param royaltyClaimDetails The details of the royalty claim from child IPs,
/// see {IRoyaltyWorkflows-RoyaltyClaimDetails}.
Expand All @@ -93,7 +90,6 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
function transferToVaultAndSnapshotAndClaimBySnapshotBatch(
address ancestorIpId,
address claimer,
address[] calldata currencyTokens,
uint256[] calldata unclaimedSnapshotIds,
RoyaltyClaimDetails[] calldata royaltyClaimDetails
) external returns (uint256 snapshotId, uint256[] memory amountsClaimed) {
Expand All @@ -113,6 +109,8 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
// Takes a snapshot of the ancestor IP's royalty vault
snapshotId = ancestorIpRoyaltyVault.snapshot();

address[] memory currencyTokens = _getCurrencyTokenList(royaltyClaimDetails);

// Claims revenue for each specified currency token from the latest snapshot
amountsClaimed = ancestorIpRoyaltyVault.claimRevenueOnBehalfByTokenBatch({
snapshotId: snapshotId,
Expand All @@ -131,7 +129,7 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
returns (uint256 claimedAmount) {
amountsClaimed[i] += claimedAmount;
} catch {
amountsClaimed[i] += 0;
// Continue to the next currency token
sebsadface marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -197,11 +195,47 @@ contract RoyaltyWorkflows is IRoyaltyWorkflows, MulticallUpgradeable, AccessMana
amountsClaimed[i] += claimedAmount;
} catch {
// Continue to the next currency token
amountsClaimed[i] += 0;
}
}
}

/// @dev Extracts all unique currency token addresses from an array of RoyaltyClaimDetails.
/// @param royaltyClaimDetails The details of the royalty claim from child IPs,
/// see {IRoyaltyWorkflows-RoyaltyClaimDetails}.
/// @return currencyTokenList An array of unique currency token addresses extracted from `royaltyClaimDetails`.
function _getCurrencyTokenList(
RoyaltyClaimDetails[] calldata royaltyClaimDetails
) private pure returns (address[] memory currencyTokenList) {
uint256 length = royaltyClaimDetails.length;
address[] memory tempUniqueTokenList = new address[](length);
uint256 uniqueCount = 0;

for (uint256 i = 0; i < length; i++) {
address currencyToken = royaltyClaimDetails[i].currencyToken;
bool isDuplicate = false;

// Check if `currencyToken` already in `tempUniqueTokenList`
for (uint256 j = 0; j < uniqueCount; j++) {
if (tempUniqueTokenList[j] == currencyToken) {
// set the `isDuplicate` flag if `currencyToken` already in `tempUniqueTokenList`
isDuplicate = true;
break;
}
}

// Add `currencyToken` to `tempUniqueTokenList` if it's not already in `tempUniqueTokenList`
if (!isDuplicate) {
tempUniqueTokenList[uniqueCount] = currencyToken;
uniqueCount++;
}
}

currencyTokenList = new address[](uniqueCount);
for (uint256 i = 0; i < uniqueCount; i++) {
currencyTokenList[i] = tempUniqueTokenList[i];
}
}

//
// Upgrade
//
Expand Down
44 changes: 25 additions & 19 deletions test/workflows/RoyaltyWorkflows.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ contract RoyaltyWorkflowsTest is BaseTest {

function test_RoyaltyWorkflows_transferToVaultAndSnapshotAndClaimByTokenBatch() public {
// setup IP graph with no snapshot
_setupIpGraph(0);
uint256 numSnapshots = 0;
_setupIpGraph(numSnapshots);

IRoyaltyWorkflows.RoyaltyClaimDetails[] memory claimDetails = new IRoyaltyWorkflows.RoyaltyClaimDetails[](4);
claimDetails[0] = IRoyaltyWorkflows.RoyaltyClaimDetails({
Expand Down Expand Up @@ -79,24 +80,23 @@ contract RoyaltyWorkflowsTest is BaseTest {
amount: (defaultMintingFeeC * defaultCommRevShareC) / royaltyModule.maxPercent() // 500 * 20% = 100
});

address[] memory currencyTokens = new address[](2);
currencyTokens[0] = address(mockTokenA);
currencyTokens[1] = address(mockTokenC);

uint256 claimerBalanceABefore = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCBefore = mockTokenC.balanceOf(u.admin);

(uint256 snapshotId, uint256[] memory amountsClaimed) = royaltyWorkflows
.transferToVaultAndSnapshotAndClaimByTokenBatch({
ancestorIpId: ancestorIpId,
claimer: u.admin,
currencyTokens: currencyTokens,
royaltyClaimDetails: claimDetails
});

uint256 claimerBalanceAAfter = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCAfter = mockTokenC.balanceOf(u.admin);

assertEq(snapshotId, numSnapshots + 1);
assertEq(amountsClaimed.length, 2); // there are 2 currency tokens
assertEq(claimerBalanceAAfter - claimerBalanceABefore, amountsClaimed[0]);
assertEq(claimerBalanceCAfter - claimerBalanceCBefore, amountsClaimed[1]);
assertEq(
claimerBalanceAAfter - claimerBalanceABefore,
defaultMintingFeeA +
Expand All @@ -109,7 +109,6 @@ contract RoyaltyWorkflowsTest is BaseTest {
royaltyModule.maxPercent() // 1000 * 10% * 10% = 10 royalty from grandChildIp
// TODO: should be 20 but MockIPGraph currently only supports single-path calculation
);

assertEq(
claimerBalanceCAfter - claimerBalanceCBefore,
defaultMintingFeeC + (defaultMintingFeeC * defaultCommRevShareC) / royaltyModule.maxPercent() // 500 from from minting fee of childIpC // 500 * 20% = 100 royalty from childIpC
Expand All @@ -118,7 +117,8 @@ contract RoyaltyWorkflowsTest is BaseTest {

function test_RoyaltyWorkflows_transferToVaultAndSnapshotAndClaimBySnapshotBatch() public {
// setup IP graph and takes 3 snapshots of ancestor IP's royalty vault
_setupIpGraph(3);
uint256 numSnapshots = 3;
_setupIpGraph(numSnapshots);

IRoyaltyWorkflows.RoyaltyClaimDetails[] memory claimDetails = new IRoyaltyWorkflows.RoyaltyClaimDetails[](4);
claimDetails[0] = IRoyaltyWorkflows.RoyaltyClaimDetails({
Expand Down Expand Up @@ -150,25 +150,24 @@ contract RoyaltyWorkflowsTest is BaseTest {
amount: (defaultMintingFeeC * defaultCommRevShareC) / royaltyModule.maxPercent() // 500 * 20% = 100
});

address[] memory currencyTokens = new address[](2);
currencyTokens[0] = address(mockTokenA);
currencyTokens[1] = address(mockTokenC);

uint256 claimerBalanceABefore = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCBefore = mockTokenC.balanceOf(u.admin);

(uint256 snapshotId, uint256[] memory amountsClaimed) = royaltyWorkflows
.transferToVaultAndSnapshotAndClaimBySnapshotBatch({
ancestorIpId: ancestorIpId,
claimer: u.admin,
currencyTokens: currencyTokens,
unclaimedSnapshotIds: unclaimedSnapshotIds,
royaltyClaimDetails: claimDetails
});

uint256 claimerBalanceAAfter = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCAfter = mockTokenC.balanceOf(u.admin);

assertEq(snapshotId, numSnapshots + 1);
assertEq(amountsClaimed.length, 2); // there are 2 currency tokens
assertEq(claimerBalanceAAfter - claimerBalanceABefore, amountsClaimed[0]);
assertEq(claimerBalanceCAfter - claimerBalanceCBefore, amountsClaimed[1]);
assertEq(
claimerBalanceAAfter - claimerBalanceABefore,
defaultMintingFeeA +
Expand All @@ -180,7 +179,6 @@ contract RoyaltyWorkflowsTest is BaseTest {
(((defaultMintingFeeA * defaultCommRevShareA) / royaltyModule.maxPercent()) * defaultCommRevShareA) /
royaltyModule.maxPercent() // 1000 * 10% * 10% = 10 royalty from grandChildIp
);

assertEq(
claimerBalanceCAfter - claimerBalanceCBefore,
defaultMintingFeeC + (defaultMintingFeeC * defaultCommRevShareC) / royaltyModule.maxPercent() // 500 from minting fee of childIpC // 500 * 20% = 100 royalty from childIpC
Expand All @@ -189,7 +187,8 @@ contract RoyaltyWorkflowsTest is BaseTest {

function test_RoyaltyWorkflows_snapshotAndClaimByTokenBatch() public {
// setup IP graph with no snapshot
_setupIpGraph(0);
uint256 numSnapshots = 0;
_setupIpGraph(numSnapshots);

address[] memory currencyTokens = new address[](2);
currencyTokens[0] = address(mockTokenA);
Expand All @@ -207,11 +206,14 @@ contract RoyaltyWorkflowsTest is BaseTest {
uint256 claimerBalanceAAfter = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCAfter = mockTokenC.balanceOf(u.admin);

assertEq(snapshotId, numSnapshots + 1);
assertEq(amountsClaimed.length, 2); // there are 2 currency tokens
assertEq(claimerBalanceAAfter - claimerBalanceABefore, amountsClaimed[0]);
assertEq(claimerBalanceCAfter - claimerBalanceCBefore, amountsClaimed[1]);
assertEq(
claimerBalanceAAfter - claimerBalanceABefore,
defaultMintingFeeA + defaultMintingFeeA // 1000 + 1000 from minting fee of childIpA and childIpB
);

assertEq(
claimerBalanceCAfter - claimerBalanceCBefore,
defaultMintingFeeC // 500 from from minting fee of childIpC
Expand All @@ -220,7 +222,8 @@ contract RoyaltyWorkflowsTest is BaseTest {

function test_RoyaltyWorkflows_snapshotAndClaimBySnapshotBatch() public {
// setup IP graph and takes 1 snapshot of ancestor IP's royalty vault
_setupIpGraph(1);
uint256 numSnapshots = 1;
_setupIpGraph(numSnapshots);

address[] memory currencyTokens = new address[](2);
currencyTokens[0] = address(mockTokenA);
Expand All @@ -239,11 +242,14 @@ contract RoyaltyWorkflowsTest is BaseTest {
uint256 claimerBalanceAAfter = mockTokenA.balanceOf(u.admin);
uint256 claimerBalanceCAfter = mockTokenC.balanceOf(u.admin);

assertEq(snapshotId, numSnapshots + 1);
assertEq(amountsClaimed.length, 2); // there are 2 currency tokens
assertEq(claimerBalanceAAfter - claimerBalanceABefore, amountsClaimed[0]);
assertEq(claimerBalanceCAfter - claimerBalanceCBefore, amountsClaimed[1]);
assertEq(
claimerBalanceAAfter - claimerBalanceABefore,
defaultMintingFeeA + defaultMintingFeeA // 1000 + 1000 from minting fee of childIpA and childIpB
);

assertEq(
claimerBalanceCAfter - claimerBalanceCBefore,
defaultMintingFeeC // 500 from from minting fee of childIpC
Expand Down Expand Up @@ -291,7 +297,7 @@ contract RoyaltyWorkflowsTest is BaseTest {
/// - `childIpC`: It has licenseTermsC attached, has 1 parent `ancestorIp`, and has 1 grandchild `grandChildIp`.
/// - `grandChildIp`: It has all 3 license terms attached. It has 3 parents and 1 grandparent IPs.
/// @param numSnapshots The number of snapshots to take of the ancestor IP's royalty vault.
function _setupIpGraph(uint32 numSnapshots) private {
function _setupIpGraph(uint256 numSnapshots) private {
uint256 ancestorTokenId = mockNft.mint(u.admin);
uint256 childTokenIdA = mockNft.mint(u.alice);
uint256 childTokenIdB = mockNft.mint(u.bob);
Expand Down
Loading