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

Jump to bottom

feat: support Shared Revenue collect module #935

Merged
merged 12 commits into from
May 21, 2024
Merged

feat: support Shared Revenue collect module #935

merged 12 commits into from
May 21, 2024

Conversation

cesarenaldi
Copy link
Member

No description provided.

@height Height
Copy link

height bot commented May 6, 2024 •
edited
Loading

This pull request has been linked to 1 task:

💡Tip: Add "Close T-20287" to the pull request title or description, to a commit message, or in a comment to mark this task as "Done" when the pull request is merged.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented May 6, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 9ffcc6d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@lens-protocol/client Minor
@lens-protocol/react-native Minor
@lens-protocol/react-web Minor
@lens-protocol/react Minor
@lens-protocol/api-bindings Patch
@lens-protocol/domain Patch
@lens-protocol/wagmi Major
@lens-protocol/blockchain-bindings Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel Vercel
Copy link

vercel bot commented May 6, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
lens-sdk-example-web-wagmi ✅ Ready (Inspect) Visit Preview May 21, 2024 9:54am

@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 0a27358 to 09c4865 Compare May 6, 2024 18:44
krzysu
krzysu reviewed May 7, 2024
View reviewed changes
packages/api-bindings/src/lens/utils/CollectModuleSettings.ts Outdated
collectNft: module.collectNft,
collectLimit: module.collectLimit,
endsAt: module.endsAt,
creatorClient: module.creatorClient ?? undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

so creatorClient is defined only if no fee?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct

krzysu
krzysu reviewed May 7, 2024
View reviewed changes
packages/api-bindings/src/lens/utils/CollectModuleSettings.ts
| gql.LegacyLimitedTimedFeeCollectModuleSettings,
): CollectFee;
function buildCollectFee(
module: Extract<CollectModuleSettings, { amount: gql.Amount }>,
Copy link
Contributor

Choose a reason for hiding this comment

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

does it read better than module: CollectModuleSettings ? or is the type safety better this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it so that works only with modules that can have a fee.

This was more type safe at the call site.

I also provided the overloads so that call site is even more accurate as I experienced a typo while I was working and wasn't flagging the call.

Finally the implementation signature just supports all possible overrides.

@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 09c4865 to 98e98bb Compare May 7, 2024 21:27
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 98e98bb to 5a92d62 Compare May 8, 2024 11:07
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 5a92d62 to 6901c33 Compare May 9, 2024 10:34
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 6901c33 to bcb477a Compare May 9, 2024 10:44
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from bcb477a to 161e5bd Compare May 9, 2024 12:22
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 161e5bd to 2787109 Compare May 9, 2024 12:30
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 2787109 to 038ec64 Compare May 9, 2024 21:14
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 038ec64 to fd8059b Compare May 10, 2024 09:58
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from fd8059b to 6c8c3c2 Compare May 10, 2024 10:50
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from 6c8c3c2 to 7272721 Compare May 13, 2024 11:33
@cesarenaldi cesarenaldi force-pushed the T-20287/share-revenue-collect-module branch from ab1b00e to 1583121 Compare May 16, 2024 12:52
@cesarenaldi cesarenaldi merged commit afb3970 into develop May 21, 2024
6 checks passed
@cesarenaldi cesarenaldi deleted the T-20287/share-revenue-collect-module branch May 21, 2024 10:53
@cesarenaldi cesarenaldi restored the T-20287/share-revenue-collect-module branch June 13, 2024 13:44
@cesarenaldi cesarenaldi deleted the T-20287/share-revenue-collect-module branch June 13, 2024 13:45
cesarenaldi added a commit that referenced this pull request Jun 13, 2024
@cesarenaldi
feat: support Shared Revenue collect module (#935)
87b457d 
* chore: removes mention of APP_NOT_ALLOWED broadcasst error reason

* feat: support new Shared Revenue Collect module

* Adopt mintFee and updates node scripts

* Supports protocolSharedRevenueCollectOpenAction input

* Verify allowance and balance of mint fee token too

* Refactors OpenAction use-case

* Updates PublicActProxy addresses

* Improves resolveCollectPolicy

* Fix EPOC value

* Ensures correct publicPaidAct amount is used and correct spender

* Simplifies use of config.sponsored

* Fix OpenActionGateway tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krzysu krzysu krzysu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cesarenaldi @krzysu