feat: add auto transfer mode #235

hhio618 · 2025-01-06T09:00:16Z

Resolves #226

src/parser/permit-generation-module.ts

0x4007 · 2025-01-06T09:22:22Z

@gentlementlegen There should be formatting checks with linter and cspell

gentlementlegen · 2025-01-06T09:30:07Z

@0x4007 Yes they seems to happen? The following run failed: https://github.com/ubiquity-os-marketplace/text-conversation-rewards/actions/runs/12629551075/job/35187679129?pr=235

This is what I see

gentlementlegen · 2025-01-07T23:39:31Z

@hhio618 I saw you marked it as [WIP] in the title, if that's the case this pull-request should be a draft so we don't review it yet.

hhio618 · 2025-01-08T05:09:33Z

@gentlementlegen I'll definitely keep that in mind for the future! For now, this task is ready for review.

src/parser/payment-module.ts

gentlementlegen · 2025-01-08T06:59:58Z

@hhio618 If you can please fix the tests:

FAIL tests/permit-generatable.test.ts
  ● Test suite failed to run

    tests/permit-generatable.test.ts:162:49 - error TS2307: Cannot find module '../src/parser/permit-generation-module' or its corresponding type declarations.

    162 const { PermitGenerationModule } = await import("../src/parser/permit-generation-module");

(cf https://github.com/ubiquity-os-marketplace/text-conversation-rewards/actions/runs/12664483458/job/35292699217?pr=235#step:5:82)

hhio618 · 2025-01-09T06:04:44Z

@gentlementlegen I'm on it! I'll update the PR once the tests are fixed.

src/parser/payment-module.ts

tests/helpers/web3.test.ts

tests/parser/payment-module.test.ts

whilefoo · 2025-01-18T19:41:14Z

@hhio618 next time please don't force push because I have to check the whole code again

src/parser/payment-module.ts

tests/fees.test.ts

tests/helpers/web3.test.ts

tests/parser/payment-module.test.ts

hhio618 · 2025-01-20T16:11:23Z

@hhio618 next time please don't force push because I have to check the whole code again

Sure, no problem! I'll avoid force-pushing next time to make reviewing easier.

ubiquity-os-beta · 2025-01-21T08:52:52Z

! No question provided

whilefoo

You still need to handle RPC retries (exponential backoff), handling reopen issue to avoid transferring multiple times and transaction failures for fatal errors like not enough funds.

For handling reopen issues, you can look at the previous comment and check transaction hashes.
For transaction failures, you proposed Multicall contract but I don't think that will work because msg.sender changes when the Multicall contract calls transfer on the ERC20 contract. @rndquu can you confirm this?

src/parser/payment-module.ts

tests/fees.test.ts

ubiquity-os-beta · 2025-01-22T17:18:12Z

! No question provided

ubiquity-os-beta · 2025-01-22T17:18:13Z

! No question provided

rndquu · 2025-01-23T09:26:39Z

For transaction failures, you proposed Multicall contract but I don't think that will work because msg.sender changes when the Multicall contract calls transfer on the ERC20 contract. @rndquu can you confirm this?

You're right, permit2 doesn't work with Multicall

whilefoo · 2025-01-23T14:54:11Z

You're right, permit2 doesn't work with Multicall

ERC20 transfer doesn't work too, right?

rndquu · 2025-01-23T16:12:43Z

You're right, permit2 doesn't work with Multicall

ERC20 transfer doesn't work too, right?

As far as I understand we're trying to solve an issue of transferring ERC20 token to multiple addresses in a single transaction.

While it's technically possible to do so via Multicall it's not safe because user has to set allowance for the Multicall contract first and only then call transferFrom(msg.sender, ...) which means that an advesary may simply call transferFrom(user, ...) which would not revert since the user has already granted allowance for the Multicall contract.

For such kind kind of task we need a contract responsible solely for transferring tokens from msg.sender. The closest thing I've found is this contract. Not sure if it's audited though. We definitely need an audited solution, there must be one somewhere in the wild.

ubiquity-os-beta · 2025-02-17T05:00:08Z

@hhio618, this task has been idle for a while. Please provide an update.

whilefoo

Did you QA this? I've just tested it and it doesn't work because when it checks allowance it fails because the ERC20 ABI doesn't contain allowance function. Next time please QA your work to avoid unnecessary reviews.

hhio618 · 2025-02-19T22:48:08Z

Did you QA this? I've just tested it and it doesn't work because when it checks allowance it fails because the ERC20 ABI doesn't contain allowance function. Next time please QA your work to avoid unnecessary reviews.

I apologize for the inconvenience. I’ve attached the QA for reference. Please let me know if there’s anything else I can assist with.

src/parser/event-incentives-module.ts

gentlementlegen · 2025-02-20T09:27:32Z

@hhio618 Would be nice to fix CI runs as well, thanks.

hhio618 · 2025-02-20T17:03:42Z

@gentlementlegen I’ve addressed the problem. Let me know if you notice anything else or if there’s anything further I can assist with.

whilefoo · 2025-02-21T09:41:38Z

src/parser/payment-module.ts

+      const { data: walletData } = await this._supabase.from("wallets").select("address").eq("id", userId).single();
+      if (!walletData?.address) {
+        this.context.logger.error("Beneficiary wallet not found");
+        return null;
+      }


Are you sure you are QA-ing correctly, I think you're just generating permits?
When testing I got this error: Beneficiary wallet not found and the error is because the id in wallets table is not an id of a user but an id of a wallet. You can check permit-generation for reference

@whilefoo I’ve double-checked this, and it’s working as expected. Here’s the transaction for QA reference: https://sepolia.etherscan.io/tx/0xec56a4d482fde94a3528e4745ea30145f0d5849fa0d8fe8cc4447952b3a068e2.

Just to note, when I attempted to close the issue for the second time, the result comment was edited. Due to the nonce being invalid, it resulted in no second transfer. Additionally, it seems the GithubCommentModule inadvertently inserted an invalid link into the edited comment.

When testing I got this error: Beneficiary wallet not found and the error is because the id in wallets table is not an id of a user but an id of a wallet. You can check permit-generation for reference

No, I didn't get that error! It might be related to your database configuration. By the way, I'll update it according to the reference.

@whilefoo You're absolutely right about the database part! It looks like my wallets/users table doesn't match the reference. I'm fixing it now!

hhio618 · 2025-02-21T13:53:00Z

@whilefoo

QA

Ubiquity-test/sandbox#13

0x4007 reviewed Jan 6, 2025

View reviewed changes

src/parser/permit-generation-module.ts Outdated Show resolved Hide resolved

src/parser/permit-generation-module.ts Outdated Show resolved Hide resolved

hhio618 force-pushed the development branch 3 times, most recently from 52b8f03 to c7d396e Compare January 7, 2025 16:28

gentlementlegen reviewed Jan 8, 2025

View reviewed changes

src/parser/payment-module.ts Outdated Show resolved Hide resolved

gentlementlegen changed the title ~~[WIP] feat: add auto transfer mode~~ feat: add auto transfer mode Jan 8, 2025

hhio618 force-pushed the development branch from 3d992e8 to 81aa774 Compare January 10, 2025 22:30

gentlementlegen requested a review from whilefoo January 12, 2025 09:51

whilefoo requested changes Jan 12, 2025

View reviewed changes

hhio618 force-pushed the development branch 2 times, most recently from acd3ea1 to a205697 Compare January 16, 2025 16:35

hhio618 requested a review from whilefoo January 17, 2025 08:37

whilefoo requested changes Jan 18, 2025

View reviewed changes

whilefoo requested changes Jan 19, 2025

View reviewed changes

tests/parser/payment-module.test.ts Outdated Show resolved Hide resolved

tests/parser/payment-module.test.ts Outdated Show resolved Hide resolved

tests/parser/payment-module.test.ts Outdated Show resolved Hide resolved

hhio618 requested a review from whilefoo January 21, 2025 09:47

whilefoo requested changes Jan 22, 2025

View reviewed changes

src/parser/payment-module.ts Outdated Show resolved Hide resolved

tests/fees.test.ts Outdated Show resolved Hide resolved

hhio618 and others added 2 commits February 13, 2025 11:14

chore: catch the InvalidNonce error

f008908

chore: updated manifest.json and dist build

e576d12

hhio618 requested a review from whilefoo February 13, 2025 16:36

ubiquity-os-beta bot marked this pull request as draft February 17, 2025 05:00

Merge remote-tracking branch 'upstream/development' into development

da95cc0

hhio618 marked this pull request as ready for review February 17, 2025 07:58

chore: updated manifest.json and dist build

c427709

whilefoo requested changes Feb 17, 2025

View reviewed changes

hhio618 mentioned this pull request Feb 19, 2025

Test the conversion-rewards: auto transfer mode Ubiquity-test/sandbox#8

Closed

hhio618 and others added 4 commits February 20, 2025 02:11

chore: add missing abi, typo fix

dc529eb

Merge remote-tracking branch 'upstream/development' into development

8f85bf9

chore: updated manifest.json and dist build

aef0e15

chore: updated manifest.json and dist build

cee9a25

hhio618 requested a review from whilefoo February 19, 2025 22:45

gentlementlegen reviewed Feb 20, 2025

View reviewed changes

src/parser/event-incentives-module.ts Outdated Show resolved Hide resolved

hhio618 and others added 2 commits February 20, 2025 09:26

chore: typo fix

5280ce1

chore: updated manifest.json and dist build

85ca74b

chore: few fixes

8103b59

whilefoo requested changes Feb 21, 2025

View reviewed changes

fix: fetch wallet from db

33db51f

github-actions bot and others added 3 commits February 21, 2025 13:54

chore: updated manifest.json and dist build

9a486bb

Merge remote-tracking branch 'upstream/development' into development

d0d8288

chore: updated manifest.json and dist build

857bdcc

hhio618 requested a review from whilefoo February 21, 2025 14:50

fix: payment-module tests

035febe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add auto transfer mode #235

feat: add auto transfer mode #235

hhio618 commented Jan 6, 2025

0x4007 commented Jan 6, 2025

gentlementlegen commented Jan 6, 2025 •

edited

Loading

gentlementlegen commented Jan 7, 2025

hhio618 commented Jan 8, 2025

gentlementlegen commented Jan 8, 2025

hhio618 commented Jan 9, 2025

whilefoo commented Jan 18, 2025

hhio618 commented Jan 20, 2025

ubiquity-os-beta bot commented Jan 21, 2025

whilefoo left a comment

ubiquity-os-beta bot commented Jan 22, 2025

ubiquity-os-beta bot commented Jan 22, 2025

rndquu commented Jan 23, 2025

whilefoo commented Jan 23, 2025

rndquu commented Jan 23, 2025

ubiquity-os-beta bot commented Feb 17, 2025

whilefoo left a comment •

edited

Loading

hhio618 commented Feb 19, 2025

gentlementlegen commented Feb 20, 2025

hhio618 commented Feb 20, 2025

whilefoo Feb 21, 2025

hhio618 Feb 21, 2025

hhio618 Feb 21, 2025

hhio618 commented Feb 21, 2025 •

edited

Loading

feat: add auto transfer mode #235

Are you sure you want to change the base?

feat: add auto transfer mode #235

Conversation

hhio618 commented Jan 6, 2025

0x4007 commented Jan 6, 2025

gentlementlegen commented Jan 6, 2025 • edited Loading

gentlementlegen commented Jan 7, 2025

hhio618 commented Jan 8, 2025

gentlementlegen commented Jan 8, 2025

hhio618 commented Jan 9, 2025

whilefoo commented Jan 18, 2025

hhio618 commented Jan 20, 2025

ubiquity-os-beta bot commented Jan 21, 2025

whilefoo left a comment

Choose a reason for hiding this comment

ubiquity-os-beta bot commented Jan 22, 2025

ubiquity-os-beta bot commented Jan 22, 2025

rndquu commented Jan 23, 2025

whilefoo commented Jan 23, 2025

rndquu commented Jan 23, 2025

ubiquity-os-beta bot commented Feb 17, 2025

whilefoo left a comment • edited Loading

Choose a reason for hiding this comment

hhio618 commented Feb 19, 2025

gentlementlegen commented Feb 20, 2025

hhio618 commented Feb 20, 2025

whilefoo Feb 21, 2025

Choose a reason for hiding this comment

hhio618 Feb 21, 2025

Choose a reason for hiding this comment

hhio618 Feb 21, 2025

Choose a reason for hiding this comment

hhio618 commented Feb 21, 2025 • edited Loading

QA

gentlementlegen commented Jan 6, 2025 •

edited

Loading

whilefoo left a comment •

edited

Loading

hhio618 commented Feb 21, 2025 •

edited

Loading