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

PromoteReferenceModule #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

allemanfredi
Copy link

@allemanfredi allemanfredi commented Apr 7, 2024

Verify my module

  • Module type: reference
  • Module name: PromoteReferenceModule
  • Module address: 0x592BC0234420fA8b023f29775a7aE44a57592c62
  • My module is immutable: yes
  • My module is using an upgradeable proxy: no
  • My module has been registered on the registry: yes
  • My module has been verified on the block explorer: yes
  • My module is a paid action so uses currencies: yes
  • Transaction hash of a successful act or actWithSig on Lens Testnet (Mumbai) contracts: https://polygonscan.com/tx/0xd8c74164460a5dacb3e26c0031981376dafa81250f7c7b1060125fce70104e18 (postWithSig)
  • Summary of my module: PromoteReferenceModule enables users to reward specific users when they quote/mirror a post.
  • I have updated the module type json file: yes

Copy link

height bot commented Apr 7, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@allemanfredi allemanfredi changed the title feat(reference-modules.json): adds PromoteReferenceModule PromoteReferenceModule Apr 7, 2024
@bigint
Copy link

bigint commented Apr 8, 2024

Is it deployed and registered on testnet? if so we can kickoff the development on Hey

@allemanfredi
Copy link
Author

Is it deployed and registered on testnet? if so we can kickoff the development on Hey

Yes! you can find it here. At the moment you can test only with WMATIC. If you need other tokens, let me know.

@bigint
Copy link

bigint commented Apr 8, 2024

Also can we get some docs on how to implement this? like params needed for encoding and decoding and its ABI something like this https://docs.madfi.xyz/guides/open-action-or-rewards-swap

@allemanfredi
Copy link
Author

Also can we get some docs on how to implement this? like params needed for encoding and decoding and its ABI something like this https://docs.madfi.xyz/guides/open-action-or-rewards-swap

You can check it here. Let me know if you need other info.

@vicnaum
Copy link

vicnaum commented Apr 25, 2024

My module is a paid action so uses currencies: no

I think this is a "yes"? Cause it uses currency transfers

@vicnaum
Copy link

vicnaum commented Apr 25, 2024

I've reviewed the module and here are some comments and suggestions:

  • It's advised to use transactionExecutor when initializing the module - that way the profile owner doesn't have to store the funds and give the approvals, which might be beneficial if the profile is on a cold/hardware wallet and will allow better integration with apps.
  • _rewards mapping is pubId=>collectorProfileId, and doesn't account for creatorProfileId. pubId might duplicate in different profiles, so someone else can overwrite values there if they have the same pubId and pass the same collectorProfileId. I suggest using profileId=>pubId=>collectorProfileId mapping.
  • Suggest to use SafeERC20 for tokens transfers

@allemanfredi
Copy link
Author

My module is a paid action so uses currencies: no

I think this is a "yes"? Cause it uses currency transfers

I thought that actions and modules were different things so I said no.

@allemanfredi
Copy link
Author

I've reviewed the module and here are some comments and suggestions:

  • It's advised to use transactionExecutor when initializing the module - that way the profile owner doesn't have to store the funds and give the approvals, which might be beneficial if the profile is on a cold/hardware wallet and will allow better integration with apps.

  • _rewards mapping is pubId=>collectorProfileId, and doesn't account for creatorProfileId. pubId might duplicate in different profiles, so someone else can overwrite values there if they have the same pubId and pass the same collectorProfileId. I suggest using profileId=>pubId=>collectorProfileId mapping.

  • Suggest to use SafeERC20 for tokens transfers

Thanks for the feedback.

Re the above:

  • Thanks I didn't know it. I'll change it.
  • I didn't know it was possible to have equals pubId.
  • thanks. I'll do it

@allemanfredi
Copy link
Author

@vicnaum Updated!

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