qt: Do not pass WalletModel* to a queued connection
ClosedPublic
Actions

Authored by PiRK on May 28 2025, 08:53.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC7d5163beff28: qt: Do not pass WalletModel* to a queued connection

Summary

On master, the following queued connection

connect(this, &RPCConsole::cmdRequest, executor, &RPCExecutor::request);

uses a const WalletModel* parameter regardless whether the ENABLE_WALLET macro is defined.

Although this code works in Qt 5, it is flawed. On Qt 6, the code gets broken because the fully defined WalletModel type is required which is not the case if ENABLE_WALLET is undefined.

Guard RPCConsole::{add,remove}Wallet() with ENABLE_WALLET

Make RPCExecutor* a member of the RPCConsole class

Do not pass WalletModel* to queued connection

This is a backport of core-gui#589
Depends on D18163

Test Plan

ninja all check-all

Run a wallet RPC command in the console

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.May 28 2025, 08:53

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 28 2025, 08:53

PiRK requested review of this revision.May 28 2025, 08:53

teamcity edited the summary of this revision. (Show Details)May 28 2025, 08:53

Fabien added a subscriber: Fabien.May 28 2025, 09:14

Fabien added inline comments.

src/qt/rpcconsole.cpp
1088 ↗	(On Diff #54224)	Should we guard against executor being null here ?

Harbormaster completed remote builds in B33461: Diff 54224.May 28 2025, 09:23

PiRK added inline comments.May 29 2025, 09:49

src/qt/rpcconsole.cpp
1088 ↗	(On Diff #54224)	It really should never be null. So I think this is a case for which we want the software to crash if it is null, rather than just silently ignoring the line. Maybe we can add a more explicit `throw` with a better error message if nullptr, to guard against actual dereferencing.