backport: bitcoin#19597, #20105, partial #18027, #20004, bitcon-core/gui#35, 71, #85, #120, #220 #5846
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PastaPastaPasta
approved these changes
Jan 29, 2024
Member
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
utACK for merge via merge commit
UdjinM6
approved these changes
Jan 30, 2024
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
light ACK, gui seems to be working correctly
BACKPORT NOTICE fixup psbt. all missing changes belongs to src/wallet/scriptpubkeyman.h/cpp ----- they are related to descriptor wallet! ------------------- 931dd47 Make lint-spelling.py happy (Glenn Willen) 11a0ffb [gui] Load PSBT from clipboard (Glenn Willen) a6cb0b0 [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen) 5dd0c03 FillPSBT: report number of inputs signed (or would sign) (Glenn Willen) 9e7b23b Improve TransactionErrorString messages. (Glenn Willen) Pull request description: Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu item, giving options to sign or broadcast the loaded PSBT as appropriate, as well as copying the result to the clipboard or saving it to a file. This is based on Sjors' bitcoin#17509, and depends on that PR going in first. (It effectively replaces the small "load PSBT" dialog from that PR with a more feature-rich one.) Some notes: * The way I display status information is maybe unusual (a status bar, rather than messageboxes.) I think it's helpful to have the information in it be persistent rather than transitory. But if people dislike it, I would probably move the "current state of the transaction" info to the top line of the main label, and the "what action just happened, and did it succeed" info into a messagebox. * I don't really know much about the translation/localization stuff. I put tr() in all the places it seemed like it ought to go. I did not attempt to translate the result of TransactionErrorString (which is shared by GUI and non-GUI code); I don't know if that's correct, but it matches the "error messages in logs should be googleable in English" heuristic. I don't know whether there are things I should be doing to reduce translator effort (like minimizing the total number of distinct message strings I use, or something.) * I don't really know how (if?) automated testing is applied to GUI code. I can make a list of PSBTs exercising all the codepaths for manual testing, if that's the right approach. Input appreciated. ACKs for top commit: instagibbs: tested ACK bitcoin@931dd47 Sjors: re-tACK 931dd47 jb55: ACK 931dd47 achow101: ACK 931dd47 Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
…ogs on Windows OS ac7ccd6 scripted-diff: Remove unused "What's This" button in dialogs on Windows (Hennadii Stepanov) b695148 qt: Add flags to prevent a "What's This" button on Windows OS (Hennadii Stepanov) Pull request description: Fix dashpay#74. From [Qt docs](https://doc.qt.io/qt-5/qdialog.html#QDialog): > The widget flags _f_ are passed on to the `QWidget` constructor. If, for example, you don't want a **What's This** button in the title bar of the dialog, pass `Qt::WindowTitleHint | Qt::WindowSystemMenuHint` in _f_. Screenshot on Windows 10 (2004): - master (3ba25e3)  - this PR (e322fe7)  ACKs for top commit: Bosch-0: tACK ac7ccd6 Tested on Windows 10.0.18363 Build 18363. promag: Code review ACK ac7ccd6 but with some suggestions. jonasschnelli: utACK ac7ccd6 Tree-SHA512: f6750a17b7203106cb4db5870becba1cef6a505d4edcc710ba131338bd3aae051510627e62c9bcb8345a7f497c614709e11aeb8f6ae3ea85967bbce2a8c69e64
88df300 qt: Do not translate file extensions (Hennadii Stepanov) Pull request description: File extensions are untranslatable by their nature. ACKs for top commit: laanwj: Concept and code review ACK 88df300 Talkless: tACK 88df300, tested on Debian Sid with Qt 5.15.2. Tested all filters except for .psbt. jarolrod: re-ACK 88df300 Tree-SHA512: 104d383543edcee8fb825f98d3b6669a7aaae2c74b6602f9bc407bf1c88be121ec535f2f9be87afa6ca775dc79865165f620553f6f6ab1d31a3f9ea93f7f9593
…ut value only once per UTXO) 82dee87 test: test decodepsbt fee calculation (count input value only once per UTXO) (Sebastian Falbesoner) Pull request description: Fixes bitcoin#19523, adding a simple test to `rpc_psbt.py` that checks that the decodepsbt fee matches the one given by the wallet (`walletcreatefundedpsbt`). This is in particular important for PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type set. Example test run after reverting commit 7512278 ("Increment input value sum only once per UTXO in decodepsbt"): ``` $ test/functional/rpc_psbt.py 2020-07-26T11:31:44.862000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__sutcd4y 20.00007580 2020-07-26T11:31:47.073000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/test_framework.py", line 118, in main self.run_test() File "test/functional/rpc_psbt.py", line 166, in run_test assert_equal(decoded['fee'], created_psbt['fee']) File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/util.py", line 49, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(20.00007580 == 0.00007580) 2020-07-26T11:31:47.125000Z TestFramework (INFO): Stopping nodes ...... ``` ACKs for top commit: achow101: ACK 82dee87 Tree-SHA512: 296b8a701f851d482ef6200c6cbf0cf0257a79a828ac6dbc39b05d8c2d839c6fdb9d3f5a084015295cfa3eac7c11faa2f2d52e619c11627b04c75150eead8330
…ion parse tests fa29b5a test: Add signet witness commitment section parse tests (MarcoFalke) fa23308 Remove gArgs global from CreateChainParams to aid testing (MarcoFalke) Pull request description: ACKs for top commit: laanwj: ACK fa29b5a Tree-SHA512: f956407d690decbfb8178bcb8f101d107389fecc3aa7be515f7b0f5ceac26d798c165100f7ddf08cec569beabcc6514862dda23b667cc4fd0a784316784735c2
1afcd41 [net] Remove CombinerAll (John Newbery) Pull request description: This was introduced in 9519a9a for use with boost signals. Boost signals have not been used in net since 8ad663c, so this code is unused. ACKs for top commit: MarcoFalke: review ACK 1afcd41 laanwj: code review ACK 1afcd41 Tree-SHA512: a4313142afb88bf12f15abc4e717b3b0d0b40d2d5db2638494af3181e1cd680d7b036087050fc0e0dfe606228849a2e20ae85135908a9ebe8ff2130f163920e1
6954156 qt: Fix visual quality of text in QR image (Hennadii Stepanov) 8071c75 qt, refactor: Limit scope of QPainter object (Hennadii Stepanov) Pull request description: Master (197450f):  This PR (6954156): - macOS 10.15.6  - Linux Mint 20  Fix dashpay#54 Fix bitcoin#19103 --- The first commit is easy to review with [`git diff --word-diff`](bitcoin-core/gui@8071c75?w=1). ACKs for top commit: jonasschnelli: Tested ACK 6954156 - tested on macOS 10.15. Fixes the problem. Tree-SHA512: 6ecb3397d2a5094c5f00ee05fc09520751568404e000a8691b6de7e57f38c2d5da628694e5e45a2b4cc302a846bbc00014c40820233eb026d3ebd4f68c2c9913
2414342 refactor: qt: Use vQueueNotifications.clear() (João Barbosa) 989e579 qt: Make transaction notification queue wallet specific (João Barbosa) 7b3b230 move-only: Define TransactionNotification before TransactionTablePriv (João Barbosa) Pull request description: Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet. This means that some transactions can be missed if multiple wallets are loaded. Fix this by having a queue for each wallet. ACKs for top commit: jonasschnelli: utACK 2414342 hebasto: ACK 2414342, I have reviewed the code and it looks OK, I agree it can be merged. ryanofsky: Code review ACK 2414342. Only change is dropping one commit Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
…ode (partial revert bitcoin#10244) 519cae8 gui: Delay interfaces::Node initialization (Russell Yanofsky) 102abff gui: Replace interface::Node references with pointers (Russell Yanofsky) 91aced7 gui: Remove unused interfaces::Node references (Russell Yanofsky) e133631 gui: Partially revert bitcoin#10244 gArgs and Params changes (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). --- This is a partial revert of bitcoin#10244. It changes gui code to go back to using gArgs and Params() functions directly instead of using interfaces::Node to handle arguments. These changes were originally pushed as part of bitcoin#19461. Motivation is to support a new GUI process connecting to an already running node process. Details are explained in commit messages, but in addition to spawning a new bitcoin-node process, we want bitcoin-gui to connect to an existing bitcoin-node process. So for that reason it should be able to parse its own parameters, rather than rely on the node. ACKs for top commit: MarcoFalke: re-ACK 519cae8, only change is rebase and addressed nits of my previous review last week 🌄 Tree-SHA512: 9c339dd82ba78bcc7b887b84d872f35ccc7dfa3d271691e6eafe8a2048cbbe3bdde1e810ce33d0714d75d048c9de3470e9e9b6f8306a6047d1cb3548f6858dc8
PastaPastaPasta
force-pushed
the
bp-v21-p19
branch
from
d59de00 to
e269fa4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Just regular bitcoin backports, mostly GUI related from v21
What was done?
Note for reviewers:
m_node.coinJoinOptions().setRoundsis removed, but actually these methods already called as I checked with debugger when settings set.m_splash. Without this fix app crashes, seems as this changes (use member variable instead local one) has been missing or incorrectly backported in the past.How Has This Been Tested?
Run unit/functional tests
Breaking Changes
N/A
Checklist: