Skip to content

Commit

Permalink
fix(payments): preventing double payments resulting from a race condi…
Browse files Browse the repository at this point in the history 
…tion

Fixed the situation where invoice and debit notes sent at the end of the
agreement resulted with double payments. While the original issue has to
be solved in yagna, we provide an integrity ensuring solution right
now.

Doubled payments can still occur, if the debit note will arrive
before the invoice. Solving this was not part of this particular change.
  • Loading branch information
@grisha87
grisha87 committed Dec 19, 2023
1 parent 1175b1c commit ffb7c15
Show file tree
Hide file tree
Showing 9 changed files with 590 additions and 195 deletions.
154 changes: 64 additions & 90 deletions package-lock.json
View file

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"node": ">=18.0.0"
},
"dependencies": {
"async-lock": "^1.4.0",
"axios": "^1.6.2",
"bottleneck": "^2.19.5",
"collect.js": "^4.36.1",
Expand All @@ -70,6 +71,7 @@
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-terser": "^0.4.4",
"@rollup/plugin-typescript": "^11.1.5",
"@types/async-lock": "^1.4.2",
"@types/eventsource": "^1.1.15",
"@types/express": "^4.17.21",
"@types/jest": "^29.5.10",
Expand Down Expand Up @@ -97,7 +99,7 @@
"supertest": "^6.3.3",
"ts-jest": "^29.1.1",
"ts-loader": "^9.5.1",
"ts-node": "^10.9.1",
"ts-node": "^10.9.2",
"tsconfig-paths": "^4.2.0",
"tslint-config-prettier": "^1.18.0",
"typedoc": "^0.25.4",
Expand Down
243 changes: 243 additions & 0 deletions src/payment/agreement_payment_process.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
import { AgreementPaymentProcess } from "./agreement_payment_process";
import { anything, instance, mock, objectContaining, reset, verify, when } from "@johanblumenberg/ts-mockito";
import { Agreement } from "../agreement";
import { Allocation } from "./allocation";
import { Invoice } from "./invoice";
import { InvoiceStatus } from "ya-ts-client/dist/ya-payment";
import { RejectionReason } from "./rejection";
import { DebitNote } from "./debit_note";

const agreementMock = mock(Agreement);
const allocationMock = mock(Allocation);
const invoiceMock = mock(Invoice);
const debitNoteMock = mock(DebitNote);

beforeEach(() => {
reset(agreementMock);
reset(allocationMock);
reset(invoiceMock);
reset(debitNoteMock);
});

describe("AgreementPaymentProcess", () => {
describe("Accepting Invoices", () => {
describe("Positive cases", () => {
it("accepts a single invoice", async () => {
when(allocationMock.id).thenReturn("1000");
when(invoiceMock.amount).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const success = await process.addInvoice(instance(invoiceMock));

expect(success).toEqual(true);
verify(invoiceMock.accept("0.123", "1000")).called();
expect(process.isFinished()).toEqual(true);
});

it("rejects invoice if it's ignored by the user defined invoice filter", async () => {
when(allocationMock.id).thenReturn("1000");
when(invoiceMock.amount).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => false,
});

const success = await process.addInvoice(instance(invoiceMock));

expect(success).toEqual(false);
verify(
invoiceMock.reject(
objectContaining({
rejectionReason: RejectionReason.RejectedByRequestorFilter,
message: "Invoice rejected by Invoice Filter",
totalAmountAccepted: "0",
}),
),
).called();
expect(process.isFinished()).toEqual(false);

Check failure on line 62 in src/payment/agreement_payment_process.test.ts

View workflow job for this annotation

GitHub Actions / Build and unit-test on supported platforms and NodeJS versions (18.x, ubuntu-latest)

AgreementPaymentProcess › Accepting Invoices › Positive cases › rejects invoice if it's ignored by the user defined invoice filter 

expect(received).toEqual(expected) // deep equality

Expected: false
Received: true

    at Object.<anonymous> (../../src/payment/agreement_payment_process.test.ts:62:38)

Check failure on line 62 in src/payment/agreement_payment_process.test.ts

View workflow job for this annotation

GitHub Actions / Build and unit-test on supported platforms and NodeJS versions (20.x, ubuntu-latest)

AgreementPaymentProcess › Accepting Invoices › Positive cases › rejects invoice if it's ignored by the user defined invoice filter 

expect(received).toEqual(expected) // deep equality

Expected: false
Received: true

    at Object.<anonymous> (../../src/payment/agreement_payment_process.test.ts:62:38)
});

it("accepts the duplicated invoice if the previous one is still not processed", async () => {
when(allocationMock.id).thenReturn("1000");
when(invoiceMock.amount).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const invoice = instance(invoiceMock);

// Simulate issue with accepting the first one
const issue = new Error("Failed to accept in yagna");
when(invoiceMock.accept("0.123", "1000"))
.thenReject(issue) // On first call
.thenResolve(); // On second call

await expect(() => process.addInvoice(invoice)).rejects.toThrow(issue);

// Then simulate the duplicate coming again
const success = await process.addInvoice(invoice);

expect(success).toEqual(true);
verify(invoiceMock.accept("0.123", "1000")).twice();
expect(process.isFinished()).toEqual(true);
});
});

describe("Negative cases", () => {
it("doesn't accept the same invoice twice if the previous one was already processed", async () => {
// TODO: False and no error to not break?
when(invoiceMock.getStatus()).thenResolve(InvoiceStatus.Accepted);

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const invoice = instance(invoiceMock);

const success = await process.addInvoice(invoice);

expect(success).toEqual(true);
expect(process.isFinished()).toEqual(true);
await expect(() => process.addInvoice(invoice)).rejects.toThrow(
"This agreement is already covered with an invoice",
);
});
});
});

describe("Accepting DebitNotes", () => {
describe("Positive cases", () => {
test("accepts a single debit note", async () => {
when(allocationMock.id).thenReturn("1000");
when(debitNoteMock.totalAmountDue).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const debitNote = instance(debitNoteMock);

const success = await process.addDebitNote(debitNote);

expect(success).toEqual(true);
verify(debitNoteMock.accept("0.123", "1000")).called();
expect(process.isFinished()).toEqual(false);
});

test("rejects debit note if it's ignored by the user defined debit note filter", async () => {
when(allocationMock.id).thenReturn("1000");
when(debitNoteMock.totalAmountDue).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => false,
invoiceFilter: () => true,
});

const debitNote = instance(debitNoteMock);

const success = await process.addDebitNote(debitNote);

expect(success).toEqual(false);
verify(
debitNoteMock.reject(
objectContaining({
rejectionReason: RejectionReason.RejectedByRequestorFilter,
message: "DebitNote rejected by DebitNote Filter",
totalAmountAccepted: "0",
}),
),
).called();
expect(process.isFinished()).toEqual(false);
});

test("rejects debit note if there is already an invoice for that process", async () => {
when(allocationMock.id).thenReturn("1000");
when(invoiceMock.amount).thenReturn("0.123");
when(debitNoteMock.totalAmountDue).thenReturn("0.456");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const invoice = instance(invoiceMock);
const debitNote = instance(debitNoteMock);

const invoiceSuccess = await process.addInvoice(invoice);
const debitNoteSuccess = await process.addDebitNote(debitNote);

expect(invoiceSuccess).toEqual(true);
verify(invoiceMock.accept("0.123", "1000")).called();

expect(debitNoteSuccess).toEqual(false);
verify(
debitNoteMock.reject(
objectContaining({
rejectionReason: RejectionReason.AgreementFinalized,
message:
"DebitNote rejected because the agreement is already covered with a final invoice that should be paid instead of the debit note",
totalAmountAccepted: "0",
}),
),
).called();
expect(process.isFinished()).toEqual(true);
});

test("accepts the duplicated debit note if the previous one is still not processed", async () => {
when(allocationMock.id).thenReturn("1000");
when(debitNoteMock.totalAmountDue).thenReturn("0.123");

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

const debitNote = instance(debitNoteMock);

// Simulate issue with accepting the first one
const issue = new Error("Failed to accept in yagna");
when(debitNoteMock.accept("0.123", "1000"))
.thenReject(issue) // On first call
.thenResolve(); // On second call

await expect(() => process.addDebitNote(debitNote)).rejects.toThrow(issue);

// Then simulate the duplicate coming again
const success = await process.addDebitNote(debitNote);

expect(success).toEqual(true);
verify(debitNoteMock.accept("0.123", "1000")).twice();
expect(process.isFinished()).toEqual(false);
});

test("doesn't accept the same debit note twice if the previous one was already processed", async () => {
when(debitNoteMock.getStatus()).thenResolve(InvoiceStatus.Accepted);

const process = new AgreementPaymentProcess(instance(agreementMock), instance(allocationMock), {
debitNoteFilter: () => true,
invoiceFilter: () => true,
});

// When
const debitNote = instance(debitNoteMock);

const firstSuccess = await process.addDebitNote(debitNote);
expect(firstSuccess).toEqual(true);

const secondSuccess = await process.addDebitNote(debitNote);
expect(secondSuccess).toEqual(false);
verify(debitNoteMock.reject(anything())).never();
expect(process.isFinished()).toEqual(false);
});
});
});
});
Loading

0 comments on commit ffb7c15

Please sign in to comment.