Skip to content

backport: v0.26 bitcoin#26690, bitcoin#28168 - #74

Open
DCG-Claude wants to merge 2 commits into
developfrom
backport-0.26-b031-src-wallet
Open

DCG-Claude wants to merge 2 commits into
developfrom
backport-0.26-b031-src-wallet

Conversation

@DCG-Claude

@DCG-Claude DCG-Claude commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Automated Bitcoin Core v0.26 backports, batch backport-0.26-b031-src-wallet.

upstream commit gates notes
bitcoin#26690 053413effc pick:pass build:pass tests:pass mech:warn tree:pass verify:pass ci_fork:pass Every upstream hunk of bitcoin#26690 lands in Dash: DatabaseCursor/DummyCursor in db.h, BerkeleyCursor and the hoisted SafeDbt
bitcoin#28168 dad37c68da build:pass tests:pass mech:warn tree:pass verify:pass Backports bitcoin#28168: UniValue::read now takes only std::string_view, and json_tests test data is generated as std::s

Skipped in this batch:

Provenance

Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.

…ject with proper return codes

4aebd83 db: Change DatabaseCursor::Next to return status enum (Andrew Chow)
d79e8dc wallet: Have cursor users use DatabaseCursor directly (Andrew Chow)
7a198bb wallet: Introduce DatabaseCursor RAII class for managing cursor (Andrew Chow)
69efbc0 Move SafeDbt out of BerkeleyBatch (Andrew Chow)

Pull request description:

  Instead of having database cursors be tied to a particular `DatabaseBatch` object and requiring its setup and teardown be separate functions in that batch, we can have cursors be separate RAII classes. This makes it easier to create and destroy cursors as well as having cursors that have slightly different behaviors.

  Additionally, since reading data from a cursor is a tri-state, this PR changes the return value of the `Next` function (formerly `ReadAtCursor`) to return an Enum rather than the current system of 2 booleans. This greatly simplifies and unifies the code that deals with cursors as now there is no confusion as to what the function returns when there are no records left to be read.

  Extracted from bitcoin#24914

ACKs for top commit:
  furszy:
    diff ACK 4aebd83
  theStack:
    Code-review ACK 4aebd83

Tree-SHA512: 5d0be56a18de5b08c777dd5a73ba5a6ef1e696fdb07d1dca952a88ded07887b7c5c04342f9a76feb2f6fe24a45dc31f094f1f5d9500e6bdf4a44f4edb66dcaa1

Dash adaptations:
- src/wallet/bdb.cpp: kept Dash's `#include <util/fs_helpers.h>` alongside the newly added `#include <util/check.h>` (Dash has already backported the fs_helpers split)
- src/wallet/bdb.cpp: Dash's file-local helper `SpanFromDbt(const BerkeleyBatch::SafeDbt&)` retargeted to the now namespace-scope `SafeDbt` — call site upstream never touched, but required by the SafeDbt move
- src/wallet/bdb.cpp: BerkeleyCursor::Next writes via Dash's `SpanFromDbt(datValue)` instead of upstream's inline `{AsBytePtr(...), get_size()}`; same bytes, keeps Dash's helper
- src/wallet/bdb.cpp: BerkeleyBatch ctor init list left as Dash has it (`: m_database(database)`); Dash already default-initializes pdb/activeTxn in the header, so upstream's `pdb(nullptr), activeTxn(nullptr)` re-add was not applied — only `m_cursor(nullptr)` had to go, and it did (member removed)
- src/wallet/bdb.h: removed Dash's `public:` nested `BerkeleyBatch::SafeDbt` (Dash had made it public for SpanFromDbt) in favour of upstream's namespace-scope SafeDbt; kept `DbTxn* activeTxn{nullptr};` default-init form while dropping `Dbc* m_cursor{nullptr};`
- src/wallet/sqlite.h: dropped `m_cursor_init` and `m_cursor_stmt` per upstream while preserving Dash-only `m_txn_started` and `m_delete_prefix_stmt` members
- src/wallet/sqlite.cpp: dropped the cursor entries from SetupSQLStatements()/Close() statement tables while keeping Dash-only `m_delete_prefix_stmt` ("delete prefix") entries; SQLiteCursor::Next writes via Dash's `SpanFromBlob()` helper instead of upstream's inline sqlite3_column_blob/bytes pair
- src/wallet/test/wallet_tests.cpp: Dash's FailBatch lives at the top of the file in an anonymous namespace and is parameterized by `m_pass` (upstream's is unparameterized and further down), so the new FailCursor carries `m_pass` too: `Next()` returns `Status::DONE` when passing and `Status::FAIL` when not. Upstream's unconditional `Status::FAIL` would break Dash's `interface_coin_lock_failed_persist` test, which requires `LoadWallet() == DBErrors::LOAD_OK` against a passing FailDatabase — the old `complete = true; return m_pass;` behaved as DONE there.
- src/wallet/dump.cpp: Dash's DumpWallet takes `WalletDatabase& db` rather than `CWallet& wallet` and has no trailing wallet-close block; the cursor hunks applied cleanly around that pre-existing divergence
@github-actions

Copy link
Copy Markdown

This pull request has conflicts, please rebase.

…from univalue

fa940f4 Remove unused raw-pointer read helper from univalue (MarcoFalke)

Pull request description:

  The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`.

  Fix both issues by removing them.

  Also, simplify the tests code by removing a `std::string` constructor where possible.

ACKs for top commit:
  stickies-v:
    utACK fa940f4
  TheCharlatan:
    tACK fa940f4

Tree-SHA512: 60c154c1046f01551335af79bf820a6104844f63e89977271b4336b3cd06ac3bab1379e18b7bc61d12bef7446029e91c16541ddecf9e88bc8bc897fc1f6ee2c8

Dash adaptations:
- src/test/script_tests.cpp: kept Dash's side (no witness comment block, no taproot/asset tests); applied only the two read_json(json_tests::script_tests) simplifications in script_build and script_json_test
- src/test/evo_trivialvalidation.cpp: Dash-only call sites built std::string(json_tests::trivially_{valid,invalid}, ... + sizeof(...)); json_tests symbols are now std::string (Makefile.test.include change), so rewritten to `const std::string& json{json_tests::...}`
- src/test/governance_validators_tests.cpp: Dash-only read_json(std::string(json_tests::proposals_{valid,invalid}, ... sizeof ...)) rewritten to read_json(json_tests::proposals_{valid,invalid}) to match the new generated std::string type
- src/wallet/test/bip39_tests.cpp: Dash-only read_json(std::string(json_tests::bip39_vectors, ... sizeof ...)) rewritten to read_json(json_tests::bip39_vectors) for the same reason
- build_msvc/test_bitcoin/test_bitcoin.vcxproj: modify/delete conflict resolved by keeping the deletion; Dash has no build_msvc/ tree (the old MSVC build system was removed), and Dash has no other MSVC project that generates json headers

Not applicable to Dash (intentionally omitted):
- build_msvc/test_bitcoin/test_bitcoin.vcxproj: Dash's tree contains no build_msvc/ at all (MSVC build system removed); the equivalent generator change (json.h -> std::string) is applied in Dash's only generator, src/Makefile.test.include
- src/test/script_standard_tests.cpp: hunk edits bip341_spk_test_vectors (TaprootBuilder test); Dash has no taproot and neither that test nor bip341_wallet_vectors.json exists in Dash
- src/test/script_tests.cpp: the bip341_keypath_test_vectors hunk edits a taproot sighash test Dash does not have (no bip341_wallet_vectors.json); the other two script_tests.cpp hunks are applied
@DCG-Claude DCG-Claude changed the title backport: v0.26 bitcoin#26690 backport: v0.26 bitcoin#26690, bitcoin#28168 Sep 24, 2026
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Potential PR merge conflicts

This is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order.

If this PR merges first

These open PRs will likely need a rebase:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant