Skip to content

Add batched claim all revenue method#435

Merged
bonnie57 merged 18 commits intodevfrom
feat/batched-claim-all-revenue
Feb 26, 2025
Merged

Add batched claim all revenue method#435
bonnie57 merged 18 commits intodevfrom
feat/batched-claim-all-revenue

Conversation

@bonnie57
Copy link
Contributor

Description

  • Add batchClaimAllRevenue method
  • Add royalty module unit tests

}

/**
* Claims all revenue from the child IPs of an ancestor IP, then transfer.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thx for adding this

});
});

it("should batch claim all revenue and convert WIP back to IP", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

and convert WIP back to IP this part isn't really tested in your assertions here. If possible, we should validate this, otherwise just remove this part so its clear we are not validating it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I noticed ClaimAllRevenue also doesn't validate, I will also remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

i remember it was hard to validate due to gas fees, if possible we should validate, but not very important imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you. It's dynamic gas fees. I have removed the description of both.

@bonnie57 bonnie57 force-pushed the feat/batched-claim-all-revenue branch from 3dc0eee to 4ae4778 Compare February 26, 2025 09:21
Copy link
Contributor

@DracoLi DracoLi left a comment

Choose a reason for hiding this comment

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

LGTM

@bonnie57 bonnie57 merged commit 094dfd0 into dev Feb 26, 2025
14 checks passed
@bonnie57 bonnie57 deleted the feat/batched-claim-all-revenue branch February 26, 2025 10:04
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