feat: meta-pn by leoslr · Pull Request #1405 · RequestNetwork/requestNetwork

leoslr · 2024-06-04T12:28:49Z

Description of the changes

Context:

In the Request object, the extensions field is of type Record<PN_ID, PN_STATE>.
As a result, the same request can't contain twice the same payment network. This is a blocker if we want a USD request to be payable in both USDC-matic and USDC-mainnet as we would need twice the pn-any-to-erc20.

Meta payment network

This PR Introduces a new payment network: META.
Its purpose is to store information related to other payment networks.
This way we do not introduce breaking changes either at the request level, or at any of the existing payment network level.

Scope

For now the sub-payment network supported are only:

any-to-erc20-proxy
any-to-eth-proxy
any-to-declarative

Supporting the erc20-fee-proxy pn would require to add a network field to it as it currently relies on the request.currency to determine the chain where the payment should happen.
I didn't want to introduce the associated changes in this PR.
Furthermore, when there are several ways of paying a request, it's most likely denominated in FIAT.

It can always be added later on if needed.

Design

State of the Meta pn:

{
  id: meta-pn,
  events: [...],
  values: {
    [key: string]: sub-pn-state // usual pn state
  } 
}

The key used to index each PN is the salt of the associated PN. This data already exists, is unique and can easily be used to identify a PN within the meta-pn extension.
Each sub-pn state contains the same data as it does when it is used as a standalone pn.

Actions of the Meta pn

This new PN has two actions:

create
apply-action-to-pn

The create action perform basic validation regarding the meta-pn input data (e.g. uniqueness of each salt). Then it uses existing payment network to create individual sub-pn-state and index them using their associated salt.

The apply-action-to-pn action applies an action to one of the sub-pn. This is required, because when we create a pn, we allow for missing information (e.g. payment address) that can be added at a later time.
If we don't have this action, it's not possible to add this information afterwards and the request will be unusable.
Same as the create action, it reuses all the capacities of existing payment network

Payment detection

To get the balance for the meta-pn extension, we get the balance events of each sub-pn, then aggregate them. Reuse all of the existing logic.

However, reusing everything means that there's a different paymentReference for each sub-pn, as the salt of each sub-pn are different AND the paymentAddress as well.
Imo this is acceptable given the fact that the protocol already planned to have several paymentReference per request (it's not the same whether we proceed to a payment or a refund see here)

Payment processor

To build the transaction for the meta-pn extension, we pass the same properties as we currently do AND the key (salt) of the pn we wish to build the transaction for. It reuses all of the existing logic.

leoslr · 2024-06-05T15:22:31Z

packages/advanced-logic/src/advanced-logic.ts

-  ): AnyToNative | undefined {
+  ):
+    | ExtensionTypes.IExtension<ExtensionTypes.PnAnyToAnyConversion.ICreationParameters>
+    | undefined {


Unrelated but align with the method above.

leoslr · 2024-06-05T15:25:10Z

packages/advanced-logic/src/extensions/abstract-extension.ts

+      copiedExtensionState[extensionAction.id] = this.applyCreation(extensionAction, timestamp, {
+        extensionsState,
+        extensionAction,
+        requestState,
+        actionSigner,
+      });


Necessary so we can call applyActionToExtension from the meta-pn context to create sub-pn states (see here).

leoslr · 2024-06-05T15:28:20Z

packages/advanced-logic/src/extensions/payment-network/meta.ts

@@ -0,0 +1,243 @@
+import { ICurrencyManager } from '@requestnetwork/currency';


File of interest

leoslr · 2024-06-05T15:28:38Z

packages/payment-detection/src/meta-payment-detector.ts

@@ -0,0 +1,190 @@
+import {


File of interest

leoslr · 2024-06-05T15:30:52Z

packages/payment-processor/test/payment/encoder-approval.test.ts

@@ -55,6 +55,7 @@ const approvalSettings = {
 const mnemonic = 'candy maple cake sugar pudding cream honey rich smooth crumble sweet treat';


There's a bit of duplication in the payment-processor tests file, but I didn't want to refactor them in this PR

leoslr · 2024-06-05T15:31:59Z

packages/types/src/advanced-logic-types.ts

-    network: ChainName,
-  ) => Extension.IExtension<Extension.PnAnyToEth.ICreationParameters> | undefined;
+    network?: ChainName,
+  ) => Extension.IExtension<Extension.PnAnyToAnyConversion.ICreationParameters> | undefined;


Unrelated. There was a type mismatch that wasn't raised by typescript before

leoslr · 2024-06-05T15:33:29Z

packages/types/src/extension-types.ts

    actionSigner: Identity.IIdentity,
    timestamp: number,
  ) => RequestLogic.IExtensionStates;
+  createCreationAction: (parameters: TCreationParameters) => IAction<TCreationParameters>;


This method is public on all PN, but it wasn't exposed in this interface.
Exposing it allows to call it here

yomarion

(partial review, to be continued) Solid work and huge improvement to the lib 🚀

packages/advanced-logic/src/extensions/payment-network/meta.ts

.../advanced-logic/test/utils/payment-network/erc20/any-to-erc20-proxy-create-data-generator.ts

packages/payment-detection/src/meta-payment-detector.ts

alexandre-abrioux

Solid work, well done!

packages/advanced-logic/test/extensions/payment-network/meta.test.ts

packages/payment-detection/src/meta-payment-detector.ts

packages/payment-processor/src/payment/utils.ts

MantisClone

Thanks for the PR @leoslr!

Here's a few comments and questions.

I've got to set it aside for today.I will continue my review tomorrow. I've only reviewed the payment-processor, types, and part of the advanced-logic. I haven't looked at payment-detection or any of the tests yet. And i'm still trying to wrap my head around the omission of the erc20-fee-proxy from the scope - especially whether the current design would prevent us from implementing it in the future.

packages/payment-detection/src/meta-payment-detector.ts

packages/advanced-logic/src/extensions/payment-network/meta.ts

packages/types/src/extensions/pn-meta.ts

packages/payment-processor/src/payment/utils.ts

packages/payment-detection/test/meta-payment-network.test.ts

packages/advanced-logic/test/extensions/payment-network/meta.test.ts

yomarion · 2024-06-14T11:15:36Z

packages/advanced-logic/test/utils/payment-network/meta-pn-data-generator.ts

+  sentRefundAmount: '0',
+});
+// actions
+export const actionCreationFull = {


Can you try to rename the fixtures in clear way so we can understand why adding a bitcoin payment is expected to throw, for instance? Example in this case createAnyToErc20Mainnet.

packages/advanced-logic/test/extensions/payment-network/meta.test.ts

packages/payment-processor/src/payment/utils.ts

yomarion · 2024-06-14T11:26:15Z

packages/payment-processor/src/payment/utils.ts

+  if (!pnIdentifier) throw new Error('Missing pn identifier');
+
+  const extensionOfInterest: ExtensionTypes.IState | undefined = pn.values[pnIdentifier];
+  if (!extensionOfInterest) throw new Error('Invalid pn identifier');


Invalid is wrong, the ID might be valid but missing from the advanced logic state.

I disagree, if the pnIdentifier is valid then it is in the advanced logic state.

Given a request, in state X. we should consider that the request is correct and does not miss data.
And the validity on pnIdentifier is based on X.

Otherwise, I can pass any pnIdentifier with any request, and it will never be considered invalid.

yomarion · 2024-06-14T11:27:57Z

packages/payment-processor/test/payment/encoder-approval.test.ts

  },
 };

+export const validMetaRequest: ClientTypes.IRequestData = {


Can't you re-use the same fixture for encoder-payment.test.ts?

The data from both test are not the same.
All the mocks should be shared in a common file, but this is already a big PR and I didn't want to include this mock refactoring as well.
Sharing the data between both file would require to update the associated tests as the expected recipient / currencies as well as

yomarion · 2024-06-14T11:29:02Z

packages/payment-processor/test/payment/encoder-payment.test.ts

+      expect(() =>
+        encodeRequestPayment(validMetaRequest, provider, {
+          conversion: alphaConversionSettings,
+          pnIdentifier: 'unknown',


This test makes it look like it throws because "unknown" is an invalid pnId, but it would also through with "eth-input" for instance, which is valid but missing.

eth-input is not valid the pnIdentifier within the meta-pn is a unique identifier to identify each sub-pn, it's not the payment network identifiers like any-to-erc20 eth-input ...

MantisClone · 2024-06-23T15:05:53Z

packages/payment-detection/src/index.ts

The payment-detection package provides a getPaymentReference() utility function.
We need a new utility function to return all the sub-paymentReferences.

MantisClone · 2024-06-23T15:10:29Z

In the Payment Detection section of the description, can you clarify which paymentReference is written on-chain when a "meta" request is paid? is it the meta paymentReference or one of the sub-pn paymentReferences?

Also, will this feature require updates to the payment-subgraph?

leoslr · 2024-07-01T08:59:15Z

@MantisClone The paymentReference used during a payment is the one associated to the sub-pn that is used to make the payment - I've updated the comment.
The payment subgraphs don't need to be updated.

leoslr added 10 commits June 4, 2024 04:45

feat: meta-pn

cff4bbe

feat: meta-pn

957a81b

revert change

af03f65

revert change

c8f847a

remove log

326c794

fixes

e440a3e

fix meta validation

e2cf57f

fix types

5712089

feat: meta-pn payment processor

a5231f2

clean

6dae29f

leoslr commented Jun 5, 2024

View reviewed changes

leoslr and others added 2 commits June 5, 2024 17:40

Merge branch 'master' into feat/meta-pn

ed837d6

fix currency types

4a15e87

leoslr marked this pull request as ready for review June 5, 2024 15:40

github-actions bot requested review from MantisClone, alexandre-abrioux, kevindavee and yomarion June 5, 2024 15:41

leoslr removed the request for review from kevindavee June 5, 2024 15:41

MantisClone requested review from aimensahnoun and sstefdev June 5, 2024 18:02

yomarion reviewed Jun 10, 2024

View reviewed changes

leoslr added 4 commits June 11, 2024 03:28

integration tests

d94e7c3

fix payment-detection tests

06ff3ba

improve mock

10934ba

fix comments and error messages

5542c70

leoslr requested a review from yomarion June 11, 2024 10:23

alexandre-abrioux approved these changes Jun 12, 2024

View reviewed changes

fix: small fixes

d465625

MantisClone reviewed Jun 13, 2024

View reviewed changes

fix comments

ee3520c

yomarion approved these changes Jun 14, 2024

View reviewed changes

MantisClone and others added 2 commits June 14, 2024 10:41

Merge branch 'master' into feat/meta-pn

4050092

update naming

3dcb548

MantisClone reviewed Jun 23, 2024

View reviewed changes

MantisClone mentioned this pull request Jun 27, 2024

Add column for payment chain in the Invoice Dashboard RequestNetwork/web-components#70

Closed

leoslr and others added 3 commits July 1, 2024 10:08

Merge branch 'master' into feat/meta-pn

4b30684

additional utility method to compute payment references for meta pn

6bcd9dc

update comment

77a3016

leoslr merged commit a9ce591 into master Jul 1, 2024

leoslr deleted the feat/meta-pn branch July 1, 2024 09:16

leoslr mentioned this pull request Jul 15, 2024

fix: declarative actions on meta pn #1427

Merged

MantisClone mentioned this pull request Jul 22, 2024

chore: publish 0.48.0 #1435

Merged

MantisClone mentioned this pull request May 28, 2025

Request SDK - Double Payment Prevention - Implement a Nonce or Idempotency Key at smart contract level #1626

Open

5 tasks

		@@ -0,0 +1,243 @@
		import { ICurrencyManager } from '@requestnetwork/currency';

		@@ -55,6 +55,7 @@ const approvalSettings = {
		const mnemonic = 'candy maple cake sugar pudding cream honey rich smooth crumble sweet treat';

Conversation

leoslr commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the changes

Context:

Meta payment network

Scope

Design

State of the Meta pn:

Actions of the Meta pn

Payment detection

Payment processor

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yomarion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexandre-abrioux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MantisClone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leoslr Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leoslr commented Jun 4, 2024 •

edited

Loading

leoslr Jun 14, 2024 •

edited

Loading

MantisClone commented Jun 23, 2024 •

edited

Loading