hub: Add an endpoint to create a scheduled payment #3231

jurgenwerk · 2022-09-16T13:08:43Z

Ticket: CS-4373

It goes like this:

Client will generate the signature for creating the transaction to add a scheduled payment on the blockchain
Client will send the scheduled payment params to the endpoint, along with the signature
Endpoint will save the scheduled payment row in the db, and submit a blockchain transaction. The request will wait until we are able to get the transaction hash. If there are problems submitting the transaction, the request will fail, and the scheduled payment row will be discarded
At this point, the transaction is not mined yet. This could take many minutes to finish, so we do it async in a background worker which will update the scheduled payment row once done (it will write the creation_block_number). This way the client can poll the scheduled payment and when creation_block_number gets written by the background job, it means the scheduled payment has been added in both blockchain and the crank - the client can then show it as "added successfully", otherwise it will be "pending".

TODO:

Test using a real chain
More thorough validator checks + validator tests
Add a route test for a recurring scheduled payment (important: see if payAt gets set correctly)
Prevent double save (at the beginning of the request, check if there was a very similar scheduled payment created very recently)

…ransaction is done

…he type of "v" in vrs to number

github-actions · 2022-09-16T13:12:24Z

ssr-web test results

    1 files ±0     1 suites ±0 11s ⏱️ -1s
172 tests ±0 172 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 9ae745a. ± Comparison against base commit 166cfb6.

github-actions · 2022-09-16T13:13:32Z

web-client test results

    1 files ±0     1 suites ±0 1m 21s ⏱️ +2s
525 tests ±0 525 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 9ae745a. ± Comparison against base commit 166cfb6.

jurgenwerk · 2022-09-16T13:21:33Z

This is ready for a draft review, I'm currently not planning any significant changes, only the things listed in TODO.

backspace · 2022-09-16T15:28:24Z

packages/hub/node-tests/routes/scheduled-payments-test.ts

+  });
+
+  it('does not persist a scheduled payment when something goes wrong with on chain call', async function () {
+    forceSchedulePaymentOnChainTimeout = true;


Maybe this should be reset to false in a beforeEach in case any tests get added after this?

Sounds good. Will change

backspace · 2022-09-16T15:31:56Z

packages/hub/routes/scheduled-payments.ts

+              // can poll the record to see when it's mined (creation_block_number attribute will be set).
+              await this.workerClient.addJob('scheduled-payment-on-chain-creation-waiter', {
+                scheduledPaymentId: scheduledPayment.id,
+              });


The profile purchase code uses a “job ticket” pattern for a slow/chain operation; the endpoint returns a newly-created ticket that the consumer can poll at /api/job-tickets/:id to track progress. Would it make sense to use that pattern here?

I thought about it. I think it's easier to just query for the scheduled payments. If creation_block_number is present on the record, it will mean the job finished successfully. In this case job ticket pattern sounds a bit roundabout

backspace · 2022-09-16T15:34:04Z

packages/hub/tasks/scheduled-payment-on-chain-creation-waiter.ts

+      });
+    }
+  }
+}


Is it worth having a couple of tests for this task?

habdelra · 2022-09-16T15:51:37Z

Love your approach, especially #4 🙏 I imagine that @FadhlanR will need to adjust the work he's done in the SDK to poll the hub for the txn instead of using the SDK waitUntilTransactionMined()?

habdelra · 2022-09-16T15:59:47Z

packages/cardpay-sdk/sdk/utils/signing-utils.ts

@@ -435,3 +435,15 @@ function sortSignaturesAsBytes(
    return [contractSignature, ownerSignature];
  }
 }
+
+export function signatureToVRS(rawSignature: string) {


is this essentially the same as ethSignSignatureToRSVForSafe() in this module?

It looks like it. However, there is one diference, in the recovery identifier (v):

signatureToVRS uses parseInt(value) while ethSignSignatureToRSVForSafe uses parseInt(value, 16)

So for example, let's have:
signature = 0x21fbf0696d5e0aa2ef41a2b4ffb623bcaf070461d61cf7251c74161f82fec3a4370854bc0a34b3ab487c1bc021cd318c734c51ae29374f2beb0e6f2dd49b4bf41c

v is the last 2 bytes and that is 1c

signatureToVRS:

parseInt("1c")
=> 1

ethSignSignatureToRSVForSafe:

parseInt("1c", 16)
=> 28

Judging by this article it looks like the v can be either either 27 (0x1b) or 28 (0x1c). That makes me think ethSignSignatureToRSVForSafe is the correct implementation and we should use it instead of signatureToVRS (and get rid of this one).

However I'm not sure how this even worked then, given the parsing might be wrong. 🙇‍♂️ Judging by this comment ethereum/go-ethereum#19751 (comment), v can also be 0 or 1 and this is the reason both implementations that we have are working.

habdelra · 2022-09-16T16:01:14Z

packages/cardpay-sdk/sdk/utils/signing-utils.ts

+}
+
+export function strip0x(s: string) {
+  return Web3.utils.isHexStrict(s) ? s.substr(2) : s;


using s.replace(/^0x/, '') is simpler and it's idempotent--you can run it as many times as you want and it will always return the correct thing.

Agree! Will change it

jurgenwerk · 2022-09-19T08:55:02Z

@habdelra I think @FadhlanR used waitUntilTransactionMined for the act of enabling the scheduled payment module on the safe. I believe we discussed that doesn't concern the hub and that we want to delegate the waiting to the client. @FadhlanR can you confirm?

FadhlanR · 2022-09-19T12:04:03Z

From what I understand for now the development in the hub is only related to the crank work which is regarding scheduling the payment and triggering the execution of the scheduled payment. I don't think we plan to use the hub to pull any type of transactions, such as creating safe and enabling modules that will not be handled by the hub/crank. CMIIW

habdelra · 2022-09-19T13:44:22Z

@jurgenwerk @FadhlanR so i'm confused them about about this function in the SDK: https://github.com/cardstack/cardstack/blob/main/packages/cardpay-sdk/sdk/scheduled-payment-module.ts#L672. This function uses waitUntilTransactionMined() https://github.com/cardstack/cardstack/blob/main/packages/cardpay-sdk/sdk/scheduled-payment-module.ts#L739. Shouldn't this function be responsible for calling the endpoint in this PR to setup the scheduled payments, and then based on the comments in #4 of this PR also poll the hub to confirm that the sceduled payments have been setup?

To be clear is this PR about establishing the schedule for the payments or to execute a specific payment that is part of the schedule?

jurgenwerk · 2022-09-26T10:18:44Z

Closing because the approach changed, and it will be split into 2 PRs - one for the SDK/CLI and one for the hub.

jurgenwerk added 7 commits September 16, 2022 14:43

Add signature field to scheduled payments table

1bccfc0

Add a background task that waits until scheduled payment blockchain t…

5875cc4

…ransaction is done

Add scheduled payment serializer and basic validator

cf853a2

Add a utility that calculates next payAt for monthly recurring payments

9d2fabd

Move signatureToVRS to utils so we can import it in the hub, change t…

6d5c746

…he type of "v" in vrs to number

Rename for consistency

5fa2f1f

Add endpoint to create a scheduled payment

9ae745a

jurgenwerk requested review from backspace, habdelra and lukemelia September 16, 2022 13:22

backspace reviewed Sep 16, 2022

View reviewed changes

habdelra reviewed Sep 16, 2022

View reviewed changes

jurgenwerk closed this Sep 26, 2022

jurgenwerk mentioned this pull request Sep 26, 2022

sdk, cli: Add scheduled payment #3257

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hub: Add an endpoint to create a scheduled payment #3231

hub: Add an endpoint to create a scheduled payment #3231

jurgenwerk commented Sep 16, 2022 •

edited

Loading

github-actions bot commented Sep 16, 2022

github-actions bot commented Sep 16, 2022

jurgenwerk commented Sep 16, 2022

backspace Sep 16, 2022

jurgenwerk Sep 19, 2022

backspace Sep 16, 2022

jurgenwerk Sep 19, 2022 •

edited

Loading

backspace Sep 16, 2022

habdelra commented Sep 16, 2022 •

edited

Loading

habdelra Sep 16, 2022

jurgenwerk Sep 19, 2022 •

edited

Loading

habdelra Sep 16, 2022 •

edited

Loading

jurgenwerk Sep 19, 2022

jurgenwerk commented Sep 19, 2022

FadhlanR commented Sep 19, 2022

habdelra commented Sep 19, 2022 •

edited

Loading

jurgenwerk commented Sep 26, 2022

hub: Add an endpoint to create a scheduled payment #3231

hub: Add an endpoint to create a scheduled payment #3231

Conversation

jurgenwerk commented Sep 16, 2022 • edited Loading

github-actions bot commented Sep 16, 2022

ssr-web test results

github-actions bot commented Sep 16, 2022

web-client test results

jurgenwerk commented Sep 16, 2022

backspace Sep 16, 2022

Choose a reason for hiding this comment

jurgenwerk Sep 19, 2022

Choose a reason for hiding this comment

backspace Sep 16, 2022

Choose a reason for hiding this comment

jurgenwerk Sep 19, 2022 • edited Loading

Choose a reason for hiding this comment

backspace Sep 16, 2022

Choose a reason for hiding this comment

habdelra commented Sep 16, 2022 • edited Loading

habdelra Sep 16, 2022

Choose a reason for hiding this comment

jurgenwerk Sep 19, 2022 • edited Loading

Choose a reason for hiding this comment

habdelra Sep 16, 2022 • edited Loading

Choose a reason for hiding this comment

jurgenwerk Sep 19, 2022

Choose a reason for hiding this comment

jurgenwerk commented Sep 19, 2022

FadhlanR commented Sep 19, 2022

habdelra commented Sep 19, 2022 • edited Loading

jurgenwerk commented Sep 26, 2022

jurgenwerk commented Sep 16, 2022 •

edited

Loading

jurgenwerk Sep 19, 2022 •

edited

Loading

habdelra commented Sep 16, 2022 •

edited

Loading

jurgenwerk Sep 19, 2022 •

edited

Loading

habdelra Sep 16, 2022 •

edited

Loading

habdelra commented Sep 19, 2022 •

edited

Loading