Skip to content

backport main changes - #2692

Merged
ethicnology merged 21 commits into
developfrom
merge/main-into-develop
Aug 19, 2026
Merged

backport main changes#2692
ethicnology merged 21 commits into
developfrom
merge/main-into-develop

Conversation

@ethicnology

Copy link
Copy Markdown
Member

No description provided.

ethicnology and others added 20 commits August 14, 2026 01:01
The exchange rejects some routes with ERR_ORD_PO404, which surfaced as a generic error and sent users to support instead of telling them what was refused.
Adds the shared PaymentNetwork primitive in lib/core/primitives so the send and receive failure families can name both networks without depending on swap's internal types.
fix(autoswap): retry a rate-limited order creation
fix(wallet): make every wallet card reachable on home
…e-message

fix(swap): name the networks when the exchange rejects a swap
fix(announcements): make the home banner fit its text
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Claude finished @ethicnology's task in 8m 3s —— View job


Review: backport main changes (PR #2692)

  • Gather context (diff vs origin/develop, migration history, CI wiring)
  • Review changes for correctness bugs, security issues, regressions
  • Post inline review comments — not possible: the gh api call to create PR review comments is blocked in this runner's permission config, so findings are inlined below with file/line anchors
  • Post summary

Summary

This merge of main into develop is largely sound. The riskiest area — restoring the published schema 14 snapshot while develop carries the Tor transport columns — is internally consistent: the columns now live only in schema 15, Schema14To15 adds them, and sqlite_database.dart:161-162 wires both steps. The new prepare_payjoin_dependency.dart tool is a well-designed fail-closed supply-chain gate (SHA-256 pinned archive and extracted binding, verified before install), and it is correctly wired into make deps, CI (flutter-setup action), and Containerfile.app. The carousel rewrite handles the empty-list, resize, and in-flight-drag edge cases and has solid test coverage. I found one migration issue worth fixing and two lower-severity notes.

Findings

1. ⚠️ Dev-channel installs at schema 14 can fail the 14→15 migration (lib/core/storage/migrations/schema_13_to_14.dart:50-62)

Removing the Tor columns from the 13→14 step is correct for fresh migrations — and it actually fixes a bug that existed on develop, where both 13→14 and 14→15 added the same columns, so a 13→15 upgrade would have crashed on the second add. However, the _addColumnIfNotExists guard was deleted along with the column adds instead of being moved: Schema14To15.migrate (lib/core/storage/migrations/schema_14_to_15.dart:9-13, unchanged by this PR) runs bare m.addColumn calls.

Failure scenario: a device that ran an earlier develop build whose 13→14 added tor_transport_mode / last_successful_tor_transport is now sitting at DB version 14 with those columns. On upgrading to a build from this branch, 14→15 throws duplicate column name and the migration fails at startup. The affected cohort is dev/internal installs only (per the updated doc comment, schema 14 never shipped in a public tag) — but those are exactly the devices develop builds are installed on.

Suggested fix: port the _addColumnIfNotExists guard into Schema14To15, mirroring the _createIfNotExists pattern this file kept. Fix this →

2. ℹ️ Payjoin binding hash vs. git ref coherence (packages/bull_payjoin/pubspec.yaml:22-26, tools/prepare_payjoin_dependency.dart:6-9)

The native side now builds from git ref 51da8ed… of SatoshiPortal/payjoin-rust, while lib/payjoin.dart is hydrated from the pub.dev rc.8 archive and pinned to that archive's binding hash. Nothing verifies the two correspond: if the pinned ref ever moves to a commit whose Rust API differs from what generated the rc.8 binding, the Dart binding and the native library drift apart — a runtime FFI mismatch no hash check catches. Worth a comment in the tool header stating the invariant ("the git ref must be the commit the rc.8 archive was published from"), so whoever bumps one knows to bump the other. Related note: the tool writes into the shared pub cache (~/.pub-cache/git/…), so the hydrated binding is visible to every checkout on the machine and is wiped by dart pub cache repair — self-healing via make deps, but worth knowing when debugging.

3. ℹ️ New l10n keys only in English and French (localization/app_en.arb:14656, localization/app_fr.arb:5046)

swapErrorRouteUnavailable / swapErrorRouteUnavailableGeneric were added to app_en.arb and app_fr.arb only; the other 25 locale files will fall back to English. If that matches the repo's translation workflow (translate later), fine — flagging in case es/pt were expected to land with the feature, since this string is user-facing on a payment failure.

What I verified (no issues found)

  • Schema/steps consistency: the regenerated sqlite_database.steps.dart column renumbering, drift_schema_v14.json, and the migration-test schema_v14.dart all agree; schema_v14_to_v15_test.dart covers the pristine upgrade path.
  • Autoswap rate-limit retry (exchange_autoswap_provider.dart): single retry, capped wait (30 s) so dispose can't stall on a server-controlled Retry-After, injectable delay for tests, and only SwapRateLimitedFailure retried. Locator call site matches the no-longer-const constructor.
  • SwapNoPaymentOptionFailure network propagation: all three _mapFailure call sites that can hit ERR_ORD_PO404 pass networks; refreshOrder's network-less call falls back to the generic message, which is fine.
  • Carousel rewrite: empty list guarded upstream (AnnouncementCarousel renders zero-height), clamp(0, length-1) safe, didChangeMetrics only realigns on genuine width change (drag-preserving behavior is directly tested), opaque Color.alphaBlend card background fixes the backdrop tint.
  • BBPullableBody.bottomInset: default 0 keeps all other callers unchanged; wallet home's 52 + 16×2 matches the actual pinned bar (52-high buttons + 16 vertical padding) plus safe-area bottom.
  • Supply-chain tool security: HTTPS download from pub.dev with SHA-256 verification of both the archive and the extracted binding before install, atomic rename into place, hard failure (exit 1) on any mismatch.

@ethicnology
ethicnology force-pushed the merge/main-into-develop branch from a462acb to 12314bd Compare August 19, 2026 15:36
@ethicnology

Copy link
Copy Markdown
Member Author

The finding is technically valid for an unreleased intermediate develop state, but that state is outside the supported migration contract. Only migrations between published schema versions are supported. The supported path is the published main v14 schema to v15, and it is covered by the normal migration tests. Development installs that used an intermediate schema may require a reset or reinstall

@ethicnology
ethicnology merged commit 4220b57 into develop Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants