(Online shop) u2u payment processing, change to “3-wallet with queuing” approach #283
Conversation
…cs with cron-job scheduler
…d script to populate existing sellers with default gas saver
adisa39
requested review from
swoocn,
omotoshoayomikun,
amt-fredrick-amoako and
DarinHajou
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi @adisa39 - I'm sure you're already aware, but It looks like the build is currently failing due to some unit tests that weren’t updated to reflect your recent changes. I will go ahead and resolve them. |
There was a problem hiding this comment.
I’ll work on tidying up the cron job setup so both your A2U process and the Sanction bot can run in parallel.
There was a problem hiding this comment.
Oh! That will be nice, thanks.
I don’t know we are still using the cron job for sanction regions implementation, they sections were all commented out before I started working on it.
|
|
||
| // workers/mongodbA2UWorker.ts | ||
| async function processNextJob(): Promise<void> { | ||
| const now = new Date(); |
|
|
||
| if (!job) return; | ||
|
|
||
| const { sellerPiUid, amount, xRef_ids, _id, attempts, memo, last_a2u_date } = job; |
There was a problem hiding this comment.
memo and last_a2u_date is not being used.
There was a problem hiding this comment.
Though it can be considered in the enqueue function, it’s is not used because of the payment batching for gas saver, that is multiple payment from different buyers with different memo descriptions.
| import mongoose, { Schema } from "mongoose"; | ||
| import { IA2UJob } from "../types"; | ||
|
|
||
| const A2UPaymentQueueSchema = new Schema<IA2UJob>( |
There was a problem hiding this comment.
Hi @adisa39 - I noticed that this DB schema doesn’t align with the PaymentQueue model outlined in the schema documentation @ https://docs.google.com/document/d/1i9JjD3veU4RmZXEiD_D9j7XvRIcZm9koJ_DFDITe1H0/edit?usp=sharing
It would have been helpful to get your feedback during the schema design phase or possibly during implementation. Just checking — was there a reason you disagreed with the proposed model, or was this an oversight? Thanks!
There was a problem hiding this comment.
Hi @swoocn - please note that various approaches have been tried before arriving at this implementation. So my thought is to raised this once execution is done and solid.
The proposed model has some repeated fields that can be accessed from other collections using foreign keys. More importantly batching of payments from different buyers with different payment details (payer_pi_username, u2a_blockchain_id, description etc.) cannot go with this feature implementation.
Please let me know your thoughts on this.
Thanks.
There was a problem hiding this comment.
Understood; no problem @adisa39!
I will make a note of this in the card and adjust the DB collection schema accordingly. 👍
| enum: ['pending', 'processing', 'completed', 'failed', 'batching'], | ||
| default: 'pending', | ||
| }, | ||
| last_a2u_date: { type: Date, default: null }, |
There was a problem hiding this comment.
Hi @adisa39 - could you clarify the difference between the last_a2u_date timestamp and the updatedAt timestamp in the context of A2U queue payment processing? Thanks, in advance!
There was a problem hiding this comment.
The last_a2u_date is used in the payment batching to determine the date of the next A2U payment for sellers with their gas saver turned on (3days from last payout). While the updatedAt keeps tracks of payment status changes while determining its position on the queue.
There was a problem hiding this comment.
Then perhaps it would make sense to rename the field to something like last_batched_a2u_date or last-gas-saver-a2u-date? Thoughts?
There was a problem hiding this comment.
Not necessary, since it’s applicable to all records and sellers with their gas saver turned off can also turned it on
There was a problem hiding this comment.
What about last_a2u_payout_date @adisa39?
| default: 'pending', | ||
| }, | ||
| last_a2u_date: { type: Date, default: null }, | ||
| attempts: { type: Number, default: 0 }, |
There was a problem hiding this comment.
Could we change the attempts field to num_retries as per the DB schema document?
There was a problem hiding this comment.
Scripts are located in the /scripts folder, which is currently hidden and excluded from commits for security reasons. These scripts are intended to be shared among developers and maintained locally, rather than being stored in the repository.
| const A2UPaymentQueueSchema = new Schema<IA2UJob>( | ||
| { | ||
| sellerPiUid: { type: String, required: true }, | ||
| amount: { type: Number, required: true }, |
There was a problem hiding this comment.
Shouldn't amount field be Types.Decimal128 to maintain data type consistency?
There was a problem hiding this comment.
This type usually difficult to handle in the BE/FE logics
There was a problem hiding this comment.
Gotcha! I can see how that may be the case. Let's stick to just Number for now then.
Included a2uPaymentQueue schema to store A2U payment and track payment status.
Included gas saver logics that batch revenue for each seller on the a2uPaymentQueue collection.
Added function to enqueue payment.
implement cron-job to automate A2U payments at 5minutes intervals.