From e9721dc13ef36790e7d5e8334b4896ce38713adb Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Thu, 9 Sep 2021 15:44:12 -0700 Subject: [PATCH 1/2] fix!: improve error message when a purse is overdrawn --- packages/ERTP/src/paymentLedger.js | 17 ++++++++++++++++- packages/ERTP/test/unitTests/test-issuerObj.js | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/ERTP/src/paymentLedger.js b/packages/ERTP/src/paymentLedger.js index f55cd737499..265ea7e383a 100644 --- a/packages/ERTP/src/paymentLedger.js +++ b/packages/ERTP/src/paymentLedger.js @@ -291,7 +291,22 @@ export const makePaymentLedger = ( */ const withdraw = (currentBalance, updatePurseBalance, amount) => { amount = coerce(amount); - const newPurseBalance = subtract(currentBalance, amount); + let newPurseBalance; + try { + newPurseBalance = subtract(currentBalance, amount); + } catch (err) { + if (err instanceof RangeError) { + const withdrawalError = Error( + // @ts-ignore `X` (aka `details`) lazily constructs a + // string, but the types do not reflect that + X`Withdrawal of ${amount} failed because the purse only contained ${currentBalance}`, + ); + assert.note(withdrawalError, X`Caused by: ${err}`); + throw withdrawalError; + } + throw err; + } + const payment = makePayment(allegedName, brand); try { // COMMIT POINT diff --git a/packages/ERTP/test/unitTests/test-issuerObj.js b/packages/ERTP/test/unitTests/test-issuerObj.js index aa8ea53dfc1..ea89fee532a 100644 --- a/packages/ERTP/test/unitTests/test-issuerObj.js +++ b/packages/ERTP/test/unitTests/test-issuerObj.js @@ -129,6 +129,22 @@ test('issuer.makeEmptyPurse', async t => { .then(checkWithdrawal); }); +test('purse.withdraw overdrawn', async t => { + t.plan(1); + const { issuer, mint, brand } = makeIssuerKit('fungible'); + const purse = issuer.makeEmptyPurse(); + const purseBalance = AmountMath.make(brand, 103980n); + const payment = mint.mintPayment(purseBalance); + purse.deposit(payment); + + const tooMuch = AmountMath.make(brand, 103981n); + + t.throws(() => purse.withdraw(tooMuch), { + message: + 'Withdrawal of {"brand":"[Alleged: fungible brand]","value":"[103981n]"} failed because the purse only contained {"brand":"[Alleged: fungible brand]","value":"[103980n]"}', + }); +}); + test('purse.deposit', async t => { t.plan(7); const { issuer, mint, brand } = makeIssuerKit('fungible'); From 859edbb1a79d0d3de70676a6eecba17768ea0cba Mon Sep 17 00:00:00 2001 From: Kate Sills Date: Thu, 9 Sep 2021 17:38:55 -0700 Subject: [PATCH 2/2] chore: assert currentBalance greater than or equal to amount to be withdrawn --- packages/ERTP/src/paymentLedger.js | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/packages/ERTP/src/paymentLedger.js b/packages/ERTP/src/paymentLedger.js index 265ea7e383a..a12d7a00b9b 100644 --- a/packages/ERTP/src/paymentLedger.js +++ b/packages/ERTP/src/paymentLedger.js @@ -291,21 +291,11 @@ export const makePaymentLedger = ( */ const withdraw = (currentBalance, updatePurseBalance, amount) => { amount = coerce(amount); - let newPurseBalance; - try { - newPurseBalance = subtract(currentBalance, amount); - } catch (err) { - if (err instanceof RangeError) { - const withdrawalError = Error( - // @ts-ignore `X` (aka `details`) lazily constructs a - // string, but the types do not reflect that - X`Withdrawal of ${amount} failed because the purse only contained ${currentBalance}`, - ); - assert.note(withdrawalError, X`Caused by: ${err}`); - throw withdrawalError; - } - throw err; - } + assert( + AmountMath.isGTE(currentBalance, amount), + X`Withdrawal of ${amount} failed because the purse only contained ${currentBalance}`, + ); + const newPurseBalance = subtract(currentBalance, amount); const payment = makePayment(allegedName, brand); try {