feat: WM on Test wallet #1427

sanducb · 2024-06-27T08:47:43Z

Context

closes [BE] Asset scale 2 to asset scale 9 #1358
closes [FE] Asset scale 2 to asset scale 9 #1357

This PR introduces changes that will allow creation of asset scale 9 wallet addresses by default and all WM accounting logic is removed as there will be no difference between payment pointers.
Rapyd will only take into account asset scale 2 (this is unchanged) while we also keep track of an asset scale 9 "virtual" balance in our wallet's db. This virtual balance is separated into incomingBalance and outgoingBalance for each PP. Whenever scale 9 incoming or outgoing transfers add up to the lowest amount representable in scale 2 (i.e. 0.01), then Rapyd balance is updated. This is similar to the previous WM accounting logic, but it now applies to all transactions.

Changes TLDR;

Allowing creation of asset scale 9 wallet addresses by default
Deleted WM accounting logic, wallet addresses are WM ready by default
Added virtual balance logic for dealing with scale 9 amounts

Important questions

Should we drop incomingBalance and outgoingBalance columns from walletAddresses table and move them to accounts table?
Should we drop debt column in the accounts table?

fix conflicts

…58-be-asset-scale-2-to-asset-scale-9

…ub.com/interledger/testnet into 1358-be-asset-scale-2-to-asset-scale-9

sanducb · 2024-07-16T11:47:26Z

docker/dev/docker-compose.yml

@@ -54,7 +54,7 @@ services:
      RATE_API_KEY: ${RATE_API_KEY}
      BASE_ASSET_SCALE: 2
      MAX_ASSET_SCALE: 9
-      WM_THRESHOLD: 100000000
+      RAPYD_THRESHOLD: 10000000 # 0.01


I modified this to 0.01 (which is the lowest non-zero amount representable in asset scale 2) instead of 0.1. Any reason to change it back?

packages/wallet/frontend/src/components/cards/AccountCard.tsx

packages/wallet/frontend/src/pages/account/[accountId].tsx

beniaminmunteanu · 2024-08-21T18:40:06Z

packages/wallet/backend/src/rapyd/rapyd-client.ts

@@ -289,7 +289,7 @@ export class RapydClient implements IRapydClient {
      : ','
    const [street, city, postCode] = args.user.address?.split(', ') ?? []
    let address = [street, postCode].join(addressDelimiter)
-    if (args.assetCode === 'EUR') address = address.replace(/[,\s]+/g, '')
+    if (args.assetCode === 'USD') address = address.replace(/[,\s]+/g, '')


not sure why this hardcoded transformation happens here tbh, but why was the asset code change needed?

Also interested in the answer here, we are creating default USD account for every new user in Rapyd, but don't know if that ihas anything to do with this. But why change EUR?

When I first started working on this issue I couldn't create an EUR account in the onboarding and thought that it should be USD by default. This was a change that I introduced in one of the first commits. Since then, I realised that I was wrong about the onboarding but forgot to revert the changes. Now I switched back to EUR 👍

beniaminmunteanu · 2024-08-21T18:44:21Z

packages/wallet/backend/src/walletAddress/service.ts

@@ -387,6 +345,7 @@ export class WalletAddressService implements IWalletAddressService {

    if (transfer.status?.status !== 'SUCCESS') {
      if (type === 'OUTGOING') {
+        //TODO this might not be needed


as in: debt being part of the walletAddress model might not be needed?

Yes, but I'm not 100% sure we should delete it. It makes sense to have it in case transferring liquidity to Rapyd fails, but also there's no mechanism to handle debt so I assume we don't care if transferring fails since the amounts are very small?

The point you are raising is valid. I think there should be a retry mechanism in place.

But I think the retry mechanism can be part of the current flow:
Transfer to rapyd fails -> the imbalance state remains the same, and the transfer should be picked up by the next iteration of the worker job and retried, since it should again be detected as imbalance.

This way, no additional mechanism requiring debt would be needed.

beniaminmunteanu · 2024-08-21T19:01:44Z

packages/wallet/frontend/src/pages/index.tsx

+  if (
+    !accountsResponse.success ||
+    !userResponse.success ||
+    !accountsResponse.result


At a quick glance, accountResponse.result might end up being an empty list.
Taking the following js truthy behaviour into account:

This might not be the desired outcome of the condition^^
Double check if this can be the case, and if such, check for accountsResponse.result.length to reach desired condition

beniaminmunteanu · 2024-08-21T19:04:12Z

packages/wallet/frontend/src/pages/index.tsx

+      account.id,
+      ctx.req.headers.cookie
+    )
+    if (walletAddressesResponse.success && walletAddressesResponse.result) {


ditto above

README.md

packages/wallet/backend/tests/mocks.ts

packages/wallet/backend/tests/incomingPayment/service.test.ts

…ub.com/interledger/testnet into 1358-be-asset-scale-2-to-asset-scale-9

sanducb · 2024-08-22T08:27:04Z

packages/wallet/frontend/src/pages/transfer/request.tsx

-  if (!accountsResponse.result) {
-    return {
-      notFound: true
+  const incomingOutgoingBalances: Record<string, number> = {}


How could we share incoming/outgoing balance state so that we don't calculate it in each page as we do here (also in send, index)? @Tymmmy @raducristianpopa

dragosp1011 · 2024-08-22T09:10:56Z

packages/wallet/backend/knexfile.js

@@ -20,6 +20,20 @@ module.exports = {
    }
  },

+  rafiki_backend: {


why is this added?

I added this so that I can migrate assets to scale 9 in the assets table which is in rafiki_backend db. That requires a separate connection.

add an env variable instead with the connection string

dragosp1011 · 2024-08-22T09:13:54Z

packages/wallet/backend/migrations/20240801155211_update_asset_scale.js

+    if (account) {
+      await knex('walletAddresses')
+        .where('id', walletAddress.id)
+        .update({ assetCode: account.assetCode })


why would asset code change?

This happens because assetCode was null for every WA for some reason and that was causing keepBalancesSynced to fail when checking that asset code exists for every WA. Until now, we only populated assetCode column on WAs table for WM payment pointers. Therefore, I changed the asset code from null to whatever asset code the account has when migrating to prevent issues.

Tymmmy · 2024-09-02T06:38:15Z

We will cherry pick the useful code we need from this.
Integrating Gatehub sandbox and replacing Rapyd, sadly made this story n0t needed anymore, as only asset code 2 will be supported.

First iteration - still WIP

01d9084

sanducb linked an issue Jun 27, 2024 that may be closed by this pull request

[BE] Asset scale 2 to asset scale 9 #1358

Closed

2 tasks

github-actions bot added type: documentation Improvements or additions to documentation package: wallet/frontend Wallet frontend implementations package: wallet/backend Wallet backend implementations type: test Improvements or additions to tests type: source Source changes labels Jun 27, 2024

sanducb changed the title ~~WM on Test wallet~~ feat: WM on Test wallet Jun 27, 2024

sanducb and others added 5 commits June 27, 2024 12:03

Format

ee0bb17

Add Rapyd threshold as env var

da749f5

Frontend fixes for WM PPs

261ce75

Update [accountId].tsx

a06af49

Update [accountId].tsx

2078190

rico191013 mentioned this pull request Jun 27, 2024

fix: conflicts of build has been resolved #1428

Merged

rico191013 and others added 12 commits June 27, 2024 18:03

fix: conflicts of build has been resolved (#1428)

c14b686

fix conflicts

Merge branch 'main' into 1358-be-asset-scale-2-to-asset-scale-9

fb4986d

Remove unused function

bb35ecf

Merge branch 'main' of https://github.com/interledger/testnet into 13…

eea0694

…58-be-asset-scale-2-to-asset-scale-9

Remove unused import

c01d1a6

Fix wallet backend tests

b3429ac

Run prettier

07a1ea3

Fix frontend formatting of asset scale

9eeeec9

Merge branch 'main' into 1358-be-asset-scale-2-to-asset-scale-9

0cbdf04

Run prettier

22c77a5

Merge branch '1358-be-asset-scale-2-to-asset-scale-9' of https://gith…

ac3be50

…ub.com/interledger/testnet into 1358-be-asset-scale-2-to-asset-scale-9

Run prettier

4876690

sanducb commented Jul 16, 2024

View reviewed changes

Final fix for displaying in diffrerent asset scales

4caaaa4

sanducb commented Jul 16, 2024

View reviewed changes

packages/wallet/frontend/src/components/cards/AccountCard.tsx Outdated Show resolved Hide resolved

sanducb commented Jul 16, 2024

View reviewed changes

packages/wallet/frontend/src/pages/account/[accountId].tsx Outdated Show resolved Hide resolved

sanducb added 7 commits August 21, 2024 18:00

Fix inconsistency in send and request screens, show scale 9

1f81cc6

Remove console log

db4de9c

Fix amounts sent to Rapyd

df198f3

Prettier

9723429

Fix type mismatch

27f32c3

Replace asset scale hardcoded values with constants

3925b62

Prettier

e81c0f5

sanducb requested a review from dragosp1011 August 21, 2024 17:43

beniaminmunteanu reviewed Aug 21, 2024

View reviewed changes

sanducb added 2 commits August 21, 2024 22:43

Fix balance calculation

650a72f

Prettier

119e4da

Tymmmy reviewed Aug 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

Tymmmy reviewed Aug 22, 2024

View reviewed changes

packages/wallet/backend/tests/mocks.ts Outdated Show resolved Hide resolved

Tymmmy reviewed Aug 22, 2024

View reviewed changes

packages/wallet/backend/tests/incomingPayment/service.test.ts Outdated Show resolved Hide resolved

Update index.tsx

b78cf23

Tymmmy previously approved these changes Aug 22, 2024

View reviewed changes

sanducb added 2 commits August 22, 2024 10:27

Revert onboarding to EUR

79fd71a

Merge branch '1358-be-asset-scale-2-to-asset-scale-9' of https://gith…

d9e4d9a

…ub.com/interledger/testnet into 1358-be-asset-scale-2-to-asset-scale-9

sanducb dismissed Tymmmy’s stale review via d9e4d9a August 22, 2024 07:29

Merge branch 'main' into 1358-be-asset-scale-2-to-asset-scale-9

939b7f4

Tymmmy approved these changes Aug 22, 2024

View reviewed changes

sanducb commented Aug 22, 2024

View reviewed changes

dragosp1011 reviewed Aug 22, 2024

View reviewed changes

Tymmmy closed this Sep 2, 2024

Tymmmy mentioned this pull request Sep 2, 2024

[GateHub]-Remove Web monetization PPs from Test Wallet - new design #1507

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: WM on Test wallet #1427

feat: WM on Test wallet #1427

sanducb commented Jun 27, 2024 •

edited

Loading

sanducb Jul 16, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024 •

edited

Loading

Tymmmy Aug 22, 2024

sanducb Aug 22, 2024

beniaminmunteanu Aug 21, 2024 •

edited

Loading

sanducb Aug 22, 2024 •

edited

Loading

beniaminmunteanu Aug 22, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024

sanducb Aug 22, 2024 •

edited

Loading

dragosp1011 Aug 22, 2024

sanducb Aug 22, 2024 •

edited

Loading

dragosp1011 Aug 22, 2024

dragosp1011 Aug 22, 2024

sanducb Aug 22, 2024 •

edited

Loading

Tymmmy commented Sep 2, 2024

feat: WM on Test wallet #1427

feat: WM on Test wallet #1427

Conversation

sanducb commented Jun 27, 2024 • edited Loading

Context

Changes TLDR;

Important questions

sanducb Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

beniaminmunteanu Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Tymmmy Aug 22, 2024

Choose a reason for hiding this comment

sanducb Aug 22, 2024

Choose a reason for hiding this comment

beniaminmunteanu Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

sanducb Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

beniaminmunteanu Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

beniaminmunteanu Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

beniaminmunteanu Aug 21, 2024

Choose a reason for hiding this comment

sanducb Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

dragosp1011 Aug 22, 2024

Choose a reason for hiding this comment

sanducb Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

dragosp1011 Aug 22, 2024

Choose a reason for hiding this comment

dragosp1011 Aug 22, 2024

Choose a reason for hiding this comment

sanducb Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Tymmmy commented Sep 2, 2024

sanducb commented Jun 27, 2024 •

edited

Loading

sanducb Jul 16, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024 •

edited

Loading

sanducb Aug 22, 2024 •

edited

Loading

beniaminmunteanu Aug 22, 2024 •

edited

Loading

beniaminmunteanu Aug 21, 2024 •

edited

Loading

sanducb Aug 22, 2024 •

edited

Loading

sanducb Aug 22, 2024 •

edited

Loading

sanducb Aug 22, 2024 •

edited

Loading