diff options
author | Florian Dold <florian@dold.me> | 2024-03-04 23:34:17 +0100 |
---|---|---|
committer | Florian Dold <florian@dold.me> | 2024-03-04 23:34:17 +0100 |
commit | 695a6a43ea143475b2dddd070a2e16680b2bc9c7 (patch) | |
tree | 87d183c71b67c5619450f4e656c9cce988715644 /packages | |
parent | e528c70159bbc147f8a947c16b60246fce65cca8 (diff) | |
download | wallet-core-695a6a43ea143475b2dddd070a2e16680b2bc9c7.tar.xz |
fix totally broken p2p coin selection
Diffstat (limited to 'packages')
-rw-r--r-- | packages/taler-util/src/http-impl.node.ts | 11 | ||||
-rw-r--r-- | packages/taler-wallet-core/src/coinSelection.test.ts | 101 | ||||
-rw-r--r-- | packages/taler-wallet-core/src/coinSelection.ts | 79 |
3 files changed, 129 insertions, 62 deletions
diff --git a/packages/taler-util/src/http-impl.node.ts b/packages/taler-util/src/http-impl.node.ts index ea2309504..b5c87843f 100644 --- a/packages/taler-util/src/http-impl.node.ts +++ b/packages/taler-util/src/http-impl.node.ts @@ -19,12 +19,8 @@ /** * Imports. */ -import { - FollowOptions, - RedirectableRequest, - http, - https, -} from "follow-redirects"; +import type { FollowOptions, RedirectableRequest } from "follow-redirects"; +import followRedirects from "follow-redirects"; import type { ClientRequest, IncomingMessage } from "node:http"; import { RequestOptions } from "node:http"; import * as net from "node:net"; @@ -45,6 +41,9 @@ import { typedArrayConcat, } from "./index.js"; +const http = followRedirects.http; +const https = followRedirects.https; + // Work around a node v20.0.0, v20.1.0, and v20.2.0 bug. The issue was fixed // in v20.3.0. // https://github.com/nodejs/node/issues/47822#issuecomment-1564708870 diff --git a/packages/taler-wallet-core/src/coinSelection.test.ts b/packages/taler-wallet-core/src/coinSelection.test.ts index 839cd22fb..4fac244fc 100644 --- a/packages/taler-wallet-core/src/coinSelection.test.ts +++ b/packages/taler-wallet-core/src/coinSelection.test.ts @@ -24,6 +24,7 @@ import { import test from "ava"; import { AvailableDenom, + PeerCoinSelectionTally, testing_greedySelectPeer, testing_selectGreedy, } from "./coinSelection.js"; @@ -42,10 +43,10 @@ const inThePast = AbsoluteTime.toProtocolTimestamp( test("p2p: should select the coin", (t) => { const instructedAmount = Amounts.parseOrThrow("LOCAL:2"); const tally = { - amountAcc: Amounts.zeroOfCurrency(instructedAmount.currency), + amountRemaining: instructedAmount, depositFeesAcc: Amounts.zeroOfCurrency(instructedAmount.currency), lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency), - }; + } satisfies PeerCoinSelectionTally; const coins = testing_greedySelectPeer( createCandidates([ { @@ -55,7 +56,6 @@ test("p2p: should select the coin", (t) => { fromExchange: "http://exchange.localhost/", }, ]), - instructedAmount, tally, ); @@ -75,7 +75,7 @@ test("p2p: should select the coin", (t) => { }); t.deepEqual(tally, { - amountAcc: Amounts.parseOrThrow("LOCAL:2"), + amountRemaining: Amounts.parseOrThrow("LOCAL:0"), depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.1"), lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"), }); @@ -84,10 +84,10 @@ test("p2p: should select the coin", (t) => { test("p2p: should select 3 coins", (t) => { const instructedAmount = Amounts.parseOrThrow("LOCAL:20"); const tally = { - amountAcc: Amounts.zeroOfCurrency(instructedAmount.currency), + amountRemaining: instructedAmount, depositFeesAcc: Amounts.zeroOfCurrency(instructedAmount.currency), lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency), - }; + } satisfies PeerCoinSelectionTally; const coins = testing_greedySelectPeer( createCandidates([ { @@ -97,7 +97,6 @@ test("p2p: should select 3 coins", (t) => { fromExchange: "http://exchange.localhost/", }, ]), - instructedAmount, tally, ); @@ -107,9 +106,9 @@ test("p2p: should select 3 coins", (t) => { denomPubHash: "hash0", maxAge: 32, contributions: [ - Amounts.parseOrThrow("LOCAL:9.9"), - Amounts.parseOrThrow("LOCAL:9.9"), - Amounts.parseOrThrow("LOCAL:0.5"), + Amounts.parseOrThrow("LOCAL:10"), + Amounts.parseOrThrow("LOCAL:10"), + Amounts.parseOrThrow("LOCAL:0.3"), ], expireDeposit: inTheDistantFuture, expireWithdraw: inTheDistantFuture, @@ -117,7 +116,7 @@ test("p2p: should select 3 coins", (t) => { }); t.deepEqual(tally, { - amountAcc: Amounts.parseOrThrow("LOCAL:20"), + amountRemaining: Amounts.parseOrThrow("LOCAL:0"), depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.3"), lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"), }); @@ -126,10 +125,10 @@ test("p2p: should select 3 coins", (t) => { test("p2p: can't select since the instructed amount is too high", (t) => { const instructedAmount = Amounts.parseOrThrow("LOCAL:60"); const tally = { - amountAcc: Amounts.zeroOfCurrency(instructedAmount.currency), + amountRemaining: instructedAmount, depositFeesAcc: Amounts.zeroOfCurrency(instructedAmount.currency), lastDepositFee: Amounts.zeroOfCurrency(instructedAmount.currency), - }; + } satisfies PeerCoinSelectionTally; const coins = testing_greedySelectPeer( createCandidates([ { @@ -139,14 +138,13 @@ test("p2p: can't select since the instructed amount is too high", (t) => { fromExchange: "http://exchange.localhost/", }, ]), - instructedAmount, tally, ); t.is(coins, undefined); t.deepEqual(tally, { - amountAcc: Amounts.parseOrThrow("LOCAL:49"), + amountRemaining: Amounts.parseOrThrow("LOCAL:10.5"), depositFeesAcc: Amounts.parseOrThrow("LOCAL:0.5"), lastDepositFee: Amounts.parseOrThrow("LOCAL:0.1"), }); @@ -246,3 +244,76 @@ function createCandidates( }; }); } + +test("p2p: regression STATER", (t) => { + const candidates = [ + { + denomPub: { + age_mask: 349441, + cipher: "RSA", + rsa_public_key: + "040000WTR9ERP6FYDM4581C1WY4DX6EA6ZP0RKDEY1VCEG1HGZQDB1E1MT0HSPWKVWYY8GN99YG8JV2BQHCV608V3AP00HZ44M4R2RDK3MEG1HY3H5VP2YESFDXC8C2J0BT6E662JJYN4MCFR8Q8ZFD7ZCA8HGBNVG4JMTS5MBDTF9CX3JC25H702K1FG2C54HR48767D18F2H11HMVK7EEF51QRGE08T704VRCNZ6WTM3Z73Z5DW4W26GBEWTDZZ4HX94HRJEH8YENXAW5T5E39TQQN7MZ7HEPB59BQWB0DDMM8MAE274BV3HC2AJVCSXFJSKBAK1B9HKERPWF7Z5556VJG6YJ9236G5SFM3RC22PJM2SXHYBWFV1WBAYF1F2026C0CM5Q3RPQETHCWZTEX8KJ2J1K904002", + }, + denomPubHash: + "TF5S4VJ8P3NN0SM5R1KW5MP665KEFMGAT2RPR70BMG0WQ5A72J53GDDE0YSCTWEXHRW8FMMX3X27RQK4D1VH69GVJBYR5RSJY3X5FS8", + feeDeposit: "STATER:1", + feeRefresh: "STATER:0", + feeRefund: "STATER:0", + feeWithdraw: "STATER:0", + stampExpireDeposit: { + t_s: 1772722025, + }, + stampExpireLegal: { + t_s: 1961938025, + }, + stampExpireWithdraw: { + t_s: 1709650025, + }, + stampStart: { + t_s: 1709045225, + }, + value: "STATER:2", + exchangeBaseUrl: "https://exchange.taler.grothoff.org/", + numAvailable: 6, + maxAge: 32, + }, + { + denomPub: { + age_mask: 349441, + cipher: "RSA", + rsa_public_key: + "040000Y84BTTQCZ28AS2KZ867V05WES3YPN34X51DNF14ADGW2HNG9YFXCCNVQ2JA9ZT3KSBD17ZN9Y71KGWAWEFYMHE0S61DW63WN58VWRXQ92440V1JSZDD7FDTYEVNGG8ZVARVZ4GGF1RCDM93R28M067S5CPRZFCCQBRFFM9YDK2W06WDXE96BDCB8MZEYPHSGK5CTDY6XJE18EMRWYRBAG0H8P6QGQS73REXX66PTJ3MRX3AK3ARZF8417QKMZZPNS1JV5EYPAC7X8R1F9G1GWAQXVVQ2XTA5NMVMNJDJ0KEM93AXD4W2C7XMVJFSQN8RVB9KZ8JXWGN1YJQK7P6476HV896THKQ05QK4F0C65P4HA7QDX84C91F42PZVMH8AMYMA2NBXEYXS0EV8NXZHMZ30JF04002", + }, + denomPubHash: + "WCMKBGR8ZKJ62YZXCRNT3EHPFQQ2M0B5CGZXW0PYA76G8PPXJMXZ7Q3WBP2DA3Z4BF21K3X9AG769RYCC39C3PT0R1DCTJA2PRTSHSR", + feeDeposit: "STATER:1", + feeRefresh: "STATER:0", + feeRefund: "STATER:0", + feeWithdraw: "STATER:0", + stampExpireDeposit: { + t_s: 1772722025, + }, + stampExpireLegal: { + t_s: 1961938025, + }, + stampExpireWithdraw: { + t_s: 1709650025, + }, + stampStart: { + t_s: 1709045225, + }, + value: "STATER:1", + exchangeBaseUrl: "https://exchange.taler.grothoff.org/", + numAvailable: 1, + maxAge: 32, + }, + ]; + const instructedAmount = Amounts.parseOrThrow("STATER:1"); + const tally = { + amountRemaining: instructedAmount, + depositFeesAcc: Amounts.parseOrThrow("STATER:0"), + lastDepositFee: Amounts.parseOrThrow("STATER:0"), + } satisfies PeerCoinSelectionTally; + const res = testing_greedySelectPeer(candidates as any, tally); + t.assert(!!res); +}); diff --git a/packages/taler-wallet-core/src/coinSelection.ts b/packages/taler-wallet-core/src/coinSelection.ts index 680e5faa1..3ece5546c 100644 --- a/packages/taler-wallet-core/src/coinSelection.ts +++ b/packages/taler-wallet-core/src/coinSelection.ts @@ -32,7 +32,6 @@ import { AllowedAuditorInfo, AllowedExchangeInfo, AmountJson, - AmountLike, Amounts, AmountString, checkDbInvariant, @@ -64,11 +63,7 @@ import { getAutoRefreshExecuteThreshold } from "./common.js"; import { DenominationRecord, WalletDbReadOnlyTransaction } from "./db.js"; import { isWithdrawableDenom } from "./denominations.js"; import { getExchangeWireDetailsInTx } from "./exchanges.js"; -import { - getDenomInfo, - InternalWalletState, - WalletExecutionContext, -} from "./wallet.js"; +import { getDenomInfo, WalletExecutionContext } from "./wallet.js"; const logger = new Logger("coinSelection.ts"); @@ -927,6 +922,9 @@ async function selectPayPeerCandidatesForExchange( ); for (const coinAvail of myExchangeCoins) { + if (coinAvail.freshCoinCount <= 0) { + continue; + } const denom = await tx.denominations.get([ coinAvail.exchangeBaseUrl, coinAvail.denomPubHash, @@ -954,8 +952,8 @@ async function selectPayPeerCandidatesForExchange( return denoms; } -interface PeerCoinSelectionTally { - amountAcc: AmountJson; +export interface PeerCoinSelectionTally { + amountRemaining: AmountJson; depositFeesAcc: AmountJson; lastDepositFee: AmountJson; } @@ -971,40 +969,37 @@ export function testing_greedySelectPeer( function greedySelectPeer( candidates: AvailableDenom[], - instructedAmount: AmountLike, tally: PeerCoinSelectionTally, ): SelResult | undefined { const selectedDenom: SelResult = {}; for (const denom of candidates) { const contributions: AmountJson[] = []; + const feeDeposit = Amounts.parseOrThrow(denom.feeDeposit); for ( let i = 0; - i < denom.numAvailable && - Amounts.cmp(tally.amountAcc, instructedAmount) < 0; + i < denom.numAvailable && Amounts.isNonZero(tally.amountRemaining); i++ ) { - const amountPayRemaining = Amounts.sub( - instructedAmount, - tally.amountAcc, + tally.depositFeesAcc = Amounts.add( + tally.depositFeesAcc, + feeDeposit, + ).amount; + tally.amountRemaining = Amounts.add( + tally.amountRemaining, + feeDeposit, ).amount; - // Maximum amount the coin could effectively contribute. - const maxCoinContrib = Amounts.sub(denom.value, denom.feeDeposit).amount; + tally.lastDepositFee = feeDeposit; - const coinSpend = Amounts.min( - Amounts.add(amountPayRemaining, denom.feeDeposit).amount, - maxCoinContrib, + const coinSpend = Amounts.max( + Amounts.min(tally.amountRemaining, denom.value), + denom.feeDeposit, ); - tally.amountAcc = Amounts.add(tally.amountAcc, coinSpend).amount; - tally.amountAcc = Amounts.sub(tally.amountAcc, denom.feeDeposit).amount; - - tally.depositFeesAcc = Amounts.add( - tally.depositFeesAcc, - denom.feeDeposit, + tally.amountRemaining = Amounts.sub( + tally.amountRemaining, + coinSpend, ).amount; - tally.lastDepositFee = Amounts.parseOrThrow(denom.feeDeposit); - contributions.push(coinSpend); } if (contributions.length > 0) { @@ -1027,14 +1022,12 @@ function greedySelectPeer( sd.contributions.push(...contributions); selectedDenom[avKey] = sd; } - if (Amounts.cmp(tally.amountAcc, instructedAmount) >= 0) { - break; - } } - if (Amounts.cmp(tally.amountAcc, instructedAmount) >= 0) { + if (Amounts.isZero(tally.amountRemaining)) { return selectedDenom; } + return undefined; } @@ -1071,8 +1064,11 @@ export async function selectPeerCoins( tx, exch.baseUrl, ); + if (logger.shouldLogTrace()) { + logger.trace(`peer payment candidate coins: ${j2s(candidates)}`); + } const tally: PeerCoinSelectionTally = { - amountAcc: Amounts.zeroOfCurrency(currency), + amountRemaining: Amounts.parseOrThrow(instructedAmount), depositFeesAcc: Amounts.zeroOfCurrency(currency), lastDepositFee: Amounts.zeroOfCurrency(currency), }; @@ -1109,8 +1105,8 @@ export async function selectPeerCoins( }); const depositFee = Amounts.parseOrThrow(denom.feeDeposit); tally.lastDepositFee = depositFee; - tally.amountAcc = Amounts.add( - tally.amountAcc, + tally.amountRemaining = Amounts.sub( + tally.amountRemaining, Amounts.sub(contrib, depositFee).amount, ).amount; tally.depositFeesAcc = Amounts.add( @@ -1120,11 +1116,13 @@ export async function selectPeerCoins( } } - const selectedDenom = greedySelectPeer( - candidates, - instructedAmount, - tally, - ); + if (logger.shouldLogTrace()) { + logger.trace(`candidates: ${j2s(candidates)}`); + logger.trace(`instructedAmount: ${j2s(instructedAmount)}`); + logger.trace(`tally: ${j2s(tally)}`); + } + + const selectedDenom = greedySelectPeer(candidates, tally); if (selectedDenom) { let minAutorefreshExecuteThreshold = TalerProtocolTimestamp.never(); @@ -1180,10 +1178,9 @@ export async function selectPeerCoins( return { type: "success", result: res }; } - const diff = Amounts.sub(instructedAmount, tally.amountAcc).amount; exchangeFeeGap[exch.baseUrl] = Amounts.add( tally.lastDepositFee, - diff, + tally.amountRemaining, ).amount; continue; |