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: add auto transfer mode #235

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

feat: add auto transfer mode #235

wants to merge 4 commits into from
+218 −49

Conversation

hhio618
Copy link

@hhio618 hhio618 commented Jan 6, 2025

Resolves #226

0x4007
0x4007 reviewed Jan 6, 2025
View reviewed changes
src/parser/permit-generation-module.ts Outdated
if (!canGeneratePermits) {
this.context.logger.error("[PermitGenerationModule] Non collaborative issue detected, skipping.");
return Promise.resolve(result);
}

const sumPayouts = await this._sumPayouts(result);
const fundingWalletBalance = await getFundingWalletBalance(this._evmNetworkId, this._erc20RewardToken, this._fudningWalletAddress);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const fundingWalletBalance = await getFundingWalletBalance(this._evmNetworkId, this._erc20RewardToken, this._fudningWalletAddress);
const fundingWalletBalance = await getFundingWalletBalance(this._evmNetworkId, this._erc20RewardToken, this._fundingWalletAddress);

src/parser/permit-generation-module.ts Outdated
const fundingWalletBalance = await getFundingWalletBalance(this._evmNetworkId, this._erc20RewardToken, this._fudningWalletAddress);

if (this._autoTransferMode && sumPayouts < fundingWalletBalance) {
this.context.logger.debug("[PermitGenerationModule] AutoTransforMode is enabled and there are enough funds, skipping.");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.context.logger.debug("[PermitGenerationModule] AutoTransforMode is enabled and there are enough funds, skipping.");
this.context.logger.debug("[PermitGenerationModule] AutoTransferMode is enabled and there are enough funds, skipping.");

@0x4007
Copy link
Member

0x4007 commented Jan 6, 2025

@gentlementlegen There should be formatting checks with linter and cspell

@gentlementlegen
Copy link
Member

gentlementlegen commented Jan 6, 2025 •
edited
Loading

@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
image

@gentlementlegen
Copy link
Member

@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
Copy link
Author

hhio618 commented Jan 8, 2025

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

gentlementlegen
gentlementlegen reviewed Jan 8, 2025
View reviewed changes
src/parser/payment-module.ts
await this._savePermitsToDatabase(result[key].userId, { issueUrl: payload.issueUrl, issueId }, permits);
} catch (e) {
this.context.logger.error(`[PermitGenerationModule] Failed to generate permits for user ${key}`, { e });
result[key].explorerUrl = `https://gnosisscan.io/tx/${tx.hash}`;
Copy link
Member

Choose a reason for hiding this comment

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

Given that we can use any network and any ERC20 address, wouldn't this be a problem?

Copy link
Author

Choose a reason for hiding this comment

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

Certainly! I addressed this by using the following:

 import { getNetworkExplorer, NetworkId } from "@ubiquity-dao/rpc-handler";

@gentlementlegen gentlementlegen changed the title [WIP] feat: add auto transfer mode feat: add auto transfer mode Jan 8, 2025
@gentlementlegen
Copy link
Member

@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
Copy link
Author

hhio618 commented Jan 9, 2025

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

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

@gentlementlegen gentlementlegen gentlementlegen left review comments

@0x4007 0x4007 0x4007 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.

Transfers then Permits
3 participants
@hhio618 @0x4007 @gentlementlegen