-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assert: "last < rowCount(parent)" in qabstractitemmodel.cpp #703
Comments
Last bit of the log and lldb backtrace, when I open a wallet via the menu:
At this point the UI is still showing the Open Wallet modal with "Opening Wallet ****" and the progress bar at the start.
|
cc @achow101: maybe this error is triggered by some data consistency issue (in the address book?) that we could perhaps catch earlier on. |
I think this has something to do with the way we're displaying the list of transactions on the overview page. It takes the full list of wallet transactions and artificially limits it to five items ( Lines 262 to 269 in 50ac8f5
The proxy filter limits the data to only show the most recent five rows by overriding the gui/src/qt/transactionfilterproxy.cpp Lines 91 to 112 in 50ac8f5
While it works for the most part, I think, strictly speaking, it violates what I suppose you could try moving line 265 to after line 268 (in |
I created a fresh Signet wallet and obtained a bunch of coins from the faucet. Receiving the 6th transaction while I was on the transaction screen did not crash, nor did returning to the home screen or restarting and then loading that wallet again. I then sent a transaction to myself (same wallet) and bumped the fee. This didn't trigger a crash, but strangely the unbumped transaction wasn't marked as conflicting either. I then waited for confirmation. It got marked as conflicted, but no crash. Going back to the home tab also did not trigger a crash. Neither did stopping and restarting the node. I proceeded to create more transactions, enough to cause the conflicted and replaced transaction to no longer be on the home page. No crash (after confirmation and restart). |
Hmm, interesting. I still think it has something to do with the (mis)use of |
I think cleaning up this particular code might just help the bug go away, so it's probably worth a try. I'll see I can run into it in another signet wallet. |
OK, I've managed to reproduce the bug by conflicting out several transactions on regtest and Signet. It turns out that I was largely correct, but missed some details. It is the case that Why did it not trigger for you, then? The source transaction list is sorted in the GUI by gui/src/qt/transactiontablemodel.cpp Lines 96 to 100 in ab98673
(Note that the comment is incorrect. The transactions are stored in the node's wallet primarily as a Lines 397 to 402 in ab98673
Anyway, any conflicted transaction(s) must have an index >= 5 when sorted by txid for the assert to trigger. An easy way to reproduce the bug is to create a transaction to yourself and fee-bump it 6 times, then wait for a block to come, which will make 6 conflicted transactions, guaranteeing that at least one will have an index >= 5. Then restart the GUI or close and re-open the wallet and it should crash. How to fix? The easiest solution is to re-order the way the filter is constructed in Edit: It'll only avoid the assert failure on startup. Any subsequent calls resulting in the proxy filter having rows removed (ie - new conflicted-out transactions) will trigger the assert. I don't think this 'solution' is sound.A more complete and correct solution is, unfortunately, more involved. There doesn't seem to be a straightforward way of limiting a I'm happy to open a PR for either approach. |
@john-moffett great find! Happy to test fixes. Not sure what the best way is. @hebasto any thoughts? |
Edited to rule out the 'simple solution', since it actually does get triggered on subsequent removals -- not by
The second solution still ought to work. |
Confirming the bug on the master branch. |
Observing the same bug on v24.0.1.
Because guix builds use release, not debug, dependencies. |
Some but not all of my (descriptor) wallets trigger the following crash, immediately when loading them.
Happens on master @ fcff639 with a
DEBUG=1
depends build, but regardless of the./configure
--debug
flag). Intel macOS 13.1. Will try some other configurations.Also tested against Homebrew qt 5.15.8 (depends is at 5.15.5), which doesn't produce the problem.
I tried bumping depends QT to 5.15.8 but this requires changing our patches, which I'm not sure how to do. Bumping QT is only a solution if the bug is in QT, not if it's on our end.
Interestingly I don't experience this bug in the v24.0.1 guix build, even though that uses the same QT version (since bitcoin/bitcoin#25719).
Random other project that ran into the same assert: https://invent.kde.org/utilities/kfind/-/merge_requests/1
The text was updated successfully, but these errors were encountered: