commit 32b6556e4169c75afbd2d29d0f10c921d052fe5c
parent 91de94f69c2cce99e425552f7560484f5486f89f
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 20 May 2026 23:05:09 +0200
fix amount arithmetic: no more floats
Diffstat:
5 files changed, 244 insertions(+), 8 deletions(-)
diff --git a/src/Amount.php b/src/Amount.php
@@ -0,0 +1,215 @@
+<?php
+
+/**
+ * @file
+ * Location: src/Amount.php
+ *
+ * Discrete representation of a GNU Taler monetary amount.
+ */
+
+namespace Drupal\taler_turnstile;
+
+/**
+ * A GNU Taler monetary amount, stored as a (currency, value, fraction)
+ * triple of discrete integers rather than a float.
+ *
+ * Taler mandates that the fractional part is a non-negative integer
+ * strictly less than 10^8, i.e. up to eight decimal digits. Float
+ * arithmetic violates this invariant (e.g. 0.1 + 0.2 = 0.30000…04),
+ * which is why every price computation that crosses an addition must
+ * go through this class.
+ *
+ * Canonical wire form is "CURRENCY:VALUE" when the fraction is zero,
+ * otherwise "CURRENCY:VALUE.FRACTION" with trailing zeros stripped
+ * from the fractional part.
+ */
+final class Amount implements \Stringable {
+
+ /**
+ * Number of fractional units per integer unit (10^8).
+ * Defined by the GNU Taler amount specification.
+ */
+ const FRAC_BASE = 100000000;
+
+ /**
+ * Maximum number of decimal digits in the fractional part.
+ */
+ const FRAC_DIGITS = 8;
+
+ public readonly string $currency;
+ public readonly int $value;
+ public readonly int $fraction;
+
+ /**
+ * @param string $currency Non-empty currency code (e.g. "EUR").
+ * @param int $value Integer part, must be >= 0.
+ * @param int $fraction Fractional part in 1/10^8 units,
+ * must be in [0, FRAC_BASE).
+ */
+ public function __construct(string $currency, int $value, int $fraction) {
+ if ($currency === '') {
+ throw new \InvalidArgumentException('Amount: currency must be non-empty.');
+ }
+ if ($value < 0) {
+ throw new \InvalidArgumentException("Amount: value must be non-negative, got $value.");
+ }
+ if ($fraction < 0 || $fraction >= self::FRAC_BASE) {
+ throw new \InvalidArgumentException(
+ "Amount: fraction must be in [0, " . self::FRAC_BASE . "), got $fraction.");
+ }
+ $this->currency = $currency;
+ $this->value = $value;
+ $this->fraction = $fraction;
+ }
+
+
+ /**
+ * Parse a canonical Taler amount string ("CUR:V" or "CUR:V.F").
+ *
+ * @throws \InvalidArgumentException if the input is not a well-formed
+ * Taler amount or its fraction exceeds 8 digits.
+ */
+ public static function parse(string $s): self {
+ if (!preg_match('/^([A-Za-z][A-Za-z0-9_-]*):(\d+)(?:\.(\d{1,8}))?$/', $s, $m)) {
+ throw new \InvalidArgumentException("Malformed Taler amount: $s");
+ }
+ return new self($m[1], (int) $m[2], self::parseFraction($m[3] ?? ''));
+ }
+
+
+ /**
+ * Build an Amount from a currency code and a non-negative decimal
+ * numeric input (string, int, or float). The numeric value must be
+ * a plain decimal — no scientific notation, no sign, no thousands
+ * separators. Up to 8 fractional digits are accepted; longer inputs
+ * are rejected rather than silently truncated.
+ *
+ * Floats are accepted for convenience but are cast via PHP's default
+ * stringification, which can introduce precision artefacts; callers
+ * that have a string already (e.g. straight out of a form value)
+ * should pass that string directly.
+ */
+ public static function fromNumeric(string $currency, string|int|float $numeric): self {
+ $s = is_string($numeric) ? trim($numeric) : (string) $numeric;
+ if (!preg_match('/^(\d+)(?:\.(\d{1,8}))?$/', $s, $m)) {
+ throw new \InvalidArgumentException(
+ "Cannot interpret '$s' as a non-negative decimal Taler amount.");
+ }
+ return new self($currency, (int) $m[1], self::parseFraction($m[2] ?? ''));
+ }
+
+
+ /**
+ * Right-pad a fractional digit string to FRAC_DIGITS and return its
+ * integer value. Empty input yields 0.
+ */
+ private static function parseFraction(string $s): int {
+ if ($s === '') {
+ return 0;
+ }
+ return (int) str_pad($s, self::FRAC_DIGITS, '0', STR_PAD_RIGHT);
+ }
+
+
+ /**
+ * Return the smallest positive amount in $currency expressible
+ * with $digits fractional digits:
+ *
+ * step(c, 0) -> c:1
+ * step(c, 1) -> c:0.1
+ * step(c, 2) -> c:0.01
+ * step(c, 8) -> c:0.00000001
+ *
+ * Use this instead of pow(0.1, $digits) for anything that compares
+ * or renders the result — pow(0.1, 2) is 0.010000000000000002,
+ * which is wrong as a currency step.
+ *
+ * @throws \InvalidArgumentException if $digits is outside
+ * [0, FRAC_DIGITS].
+ */
+ public static function step(string $currency, int $digits): self {
+ if ($digits < 0 || $digits > self::FRAC_DIGITS) {
+ throw new \InvalidArgumentException(
+ "Amount::step: digits must be in [0, " . self::FRAC_DIGITS . "], got $digits.");
+ }
+ if ($digits === 0) {
+ return new self($currency, 1, 0);
+ }
+ // FRAC_BASE = 10^FRAC_DIGITS, so the smallest unit at $digits
+ // fractional digits sits at fraction = 10^(FRAC_DIGITS - $digits).
+ // All exponents in [0, 8] yield results that fit comfortably in
+ // PHP's int, so the (int) cast on integer-valued pow() is exact.
+ return new self($currency, 0, (int) (10 ** (self::FRAC_DIGITS - $digits)));
+ }
+
+
+ /**
+ * Add another Amount. Returns a new Amount; this instance is
+ * unchanged. Both Amounts must use the same currency.
+ *
+ * @throws \DomainException on currency mismatch.
+ */
+ public function add(Amount $other): self {
+ if ($this->currency !== $other->currency) {
+ throw new \DomainException(sprintf(
+ 'Cannot add Taler amounts of different currencies (%s vs %s).',
+ $this->currency,
+ $other->currency
+ ));
+ }
+ $fraction = $this->fraction + $other->fraction;
+ $value = $this->value + $other->value;
+ if ($fraction >= self::FRAC_BASE) {
+ $fraction -= self::FRAC_BASE;
+ $value++;
+ }
+ return new self($this->currency, $value, $fraction);
+ }
+
+
+ /**
+ * Structural equality. Two Amounts compare equal iff currency,
+ * value, and fraction all match exactly.
+ */
+ public function equals(Amount $other): bool {
+ return $this->currency === $other->currency
+ && $this->value === $other->value
+ && $this->fraction === $other->fraction;
+ }
+
+
+ /**
+ * Render just the numeric part — "V" when the fraction is zero,
+ * otherwise "V.F" with trailing zeros stripped from F. The currency
+ * code is NOT included; use this when feeding callers (HTML form
+ * widgets, etc.) that want a bare decimal.
+ *
+ * Because the fraction==0 case is handled by an early return, the
+ * decimal-point branch always has at least one non-zero fractional
+ * digit and the rtrim() is safe.
+ */
+ public function numericString(): string {
+ if ($this->fraction === 0) {
+ return (string) $this->value;
+ }
+ $frac = rtrim(
+ str_pad((string) $this->fraction, self::FRAC_DIGITS, '0', STR_PAD_LEFT),
+ '0'
+ );
+ return $this->value . '.' . $frac;
+ }
+
+
+ /**
+ * Render in canonical Taler form, kept as short as possible:
+ *
+ * - fraction == 0 -> "CUR:V" (no decimal point, never "CUR:V.0")
+ * - fraction != 0 -> "CUR:V.F" with trailing zeros stripped
+ * from F ("EUR:0.5", not
+ * "EUR:0.50000000").
+ */
+ public function __toString(): string {
+ return $this->currency . ':' . $this->numericString();
+ }
+
+}
diff --git a/src/Entity/TurnstilePriceCategory.php b/src/Entity/TurnstilePriceCategory.php
@@ -10,6 +10,7 @@ namespace Drupal\taler_turnstile\Entity;
use Drupal\Core\Config\Entity\ConfigEntityBase;
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\taler_turnstile\Amount;
use Drupal\taler_turnstile\TalerBackendResult;
/**
@@ -255,6 +256,10 @@ class TurnstilePriceCategory extends ConfigEntityBase {
}
foreach ($currencyMap as $currencyCode => $price) {
$inputs = [];
+ // All amounts go through Amount::fromNumeric / Amount::add to
+ // avoid float-precision artefacts (0.1 + 0.2 = 0.30000…04)
+ // and to canonicalize the wire form across all three branches.
+ $pay_amount = Amount::fromNumeric($currencyCode, (string) $price);
if ("%none%" !== $tokenFamilySlug)
{
$inputs[] = [
@@ -272,14 +277,18 @@ class TurnstilePriceCategory extends ConfigEntityBase {
['@currency' => $currencyCode]
);
$choices[] = [
- 'amount' => $currencyCode . ':' . $price,
+ 'amount' => (string) $pay_amount,
'description' => 'Pay in ' . $currencyCode . ' with subscription',
'description_i18n' => $description_i18n,
'inputs' => $inputs,
];
$subscription_price = $this->getSubscriptionPrice ($tokenFamilySlug, $currencyCode);
if ($subscription_price !== NULL) {
- // This subscription can be bought.
+ // This subscription can be bought: charge the article
+ // price plus the subscription's purchase price, computed
+ // as a discrete integer addition.
+ $buy_amount = $pay_amount->add(
+ Amount::fromNumeric($currencyCode, (string) $subscription_price));
$outputs = [];
$outputs[] = [
'type' => 'token',
@@ -294,8 +303,7 @@ class TurnstilePriceCategory extends ConfigEntityBase {
['@currency' => $currencyCode]
);
$choices[] = [
- 'amount' => $currencyCode . ':' . ((float) $subscription_price +
- (float) $price),
+ 'amount' => (string) $buy_amount,
'description' => $description,
'description_i18n' => $description_i18n,
'outputs' => $outputs,
@@ -312,7 +320,7 @@ class TurnstilePriceCategory extends ConfigEntityBase {
['@currency' => $currencyCode]
);
$choices[] = [
- 'amount' => $currencyCode . ':' . (float) $price,
+ 'amount' => (string) $pay_amount,
'description' => $description,
'description_i18n' => $description_i18n,
'inputs' => $inputs,
diff --git a/src/Form/PriceCategoryForm.php b/src/Form/PriceCategoryForm.php
@@ -127,7 +127,11 @@ class PriceCategoryForm extends EntityForm {
'#title' => $currency_label,
'#default_value' => $existing_prices[$subscription_id][$currency_code] ?? '',
'#min' => 0,
- '#step' => $currency['step'] ?? 0.01,
+ // HTML's step attribute wants a bare decimal ("0.01"),
+ // not a Taler amount ("EUR:0.01").
+ '#step' => isset($currency['step'])
+ ? $currency['step']->numericString()
+ : '0.01',
'#size' => 20,
'#description' => $this->t('Leave empty for no price.'),
];
diff --git a/src/Form/SubscriptionPricesForm.php b/src/Form/SubscriptionPricesForm.php
@@ -144,7 +144,11 @@ class SubscriptionPricesForm extends ConfigFormBase {
'#title' => $currency_label,
'#default_value' => $existing_prices[$subscription_id][$currency_code] ?? '',
'#min' => 0,
- '#step' => $currency['step'] ?? 0.01,
+ // HTML's step attribute wants a bare decimal ("0.01"),
+ // not a Taler amount ("EUR:0.01").
+ '#step' => isset($currency['step'])
+ ? $currency['step']->numericString()
+ : '0.01',
'#size' => 20,
'#description' => $this->t('Leave empty to prevent buying this subscription with @currency.', [
'@currency' => $currency_code,
diff --git a/src/TalerMerchantApiService.php b/src/TalerMerchantApiService.php
@@ -543,11 +543,16 @@ class TalerMerchantApiService {
}
$result = array_map(function ($currency) {
+ // Amount::step gives an exact Amount for the smallest (usually) user-enterable
+ // unit given the number of fractional digits from the currency spec.
return [
'code' => $currency['currency'],
'name' => $currency['name'],
'label' => $currency['alt_unit_names'][0] ?? $currency['id'],
- 'step' => pow(0.1, $currency['num_fractional_input_digits'] ?? 2),
+ 'step' => Amount::step(
+ $currency['currency'],
+ $currency['num_fractional_input_digits'] ?? 2
+ ),
];
}, $body['currencies']);