qt: Assert QMetaObject::invokeMethod result #16348

promag · 2019-07-06T16:22:08Z

Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the invokeMethod overload that allows connecting to lambdas or member pointers, which are compile checked.

For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5.

DrahtBot · 2019-07-06T18:23:54Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16349 (qt: Remove redundant WalletController::addWallet signal by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto · 2019-07-06T19:22:42Z

@promag

Once we bump Qt to at least 5.10 these can be refactored to use the invokeMethod overload that allows connecting to lambdas or member pointers, which are compile checked.

IMO, in the long run it will be better to get rid of QMetaObject::invokeMethod completely, because QMetaObject class

... is not normally required for application programming...

promag · 2019-07-06T19:43:08Z

@hebasto since Qt 5.10 invokeMethod is very useful to run code (a)synchronous in other thread.

hebasto · 2019-07-07T08:35:12Z

@promag

since Qt 5.10 invokeMethod is very useful to run code (a)synchronous in other thread.

I believe it is possible and convenient to handle all inter-thread communications via signal-slot connections. That is the main Qt feature ;)

promag · 2019-07-07T09:06:09Z

Yes, but there's nothing wrong with convenience, that's why those were added.

hebasto · 2019-07-07T09:10:23Z

Concept ACK

laanwj · 2019-07-08T14:19:29Z

So to be clear, before concept ACK: these can only fail when the name / method signature is wrong, not for run-time reasons such as a full queue ?

promag · 2019-07-08T14:25:50Z

@laanwj yes, from the docs:

Invokes the member (a signal or a slot name) on the object obj. Returns true if the member could be invoked. Returns false if there is no such member or the parameters did not match.

such as a full queue

Is this even possible?

laanwj · 2019-07-08T15:31:58Z

Is this even possible?

I don't think so, not sure, my experience (with accidentally not rate-limiting notifications) is that the queue is very deep at least
but thanks for quoting the doc that's good enough for me.

concept and code review ACK 2fd7d0b

hebasto · 2019-07-08T15:45:10Z

This approach was used in a720a98 already.

hebasto · 2019-07-08T16:19:28Z

Tested on Debian 9.9 (system Qt 5.7.1):

$ ./src/qt/bitcoin-qt -regtest -nowallet -debug=qt

Open a wallet from menu...

bitcoin-qt: qt/walletcontroller.cpp:125: WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet>): Assertion `invoked' failed.
Aborted

$ tail -5 ~/.bitcoin/regtest/debug.log 
2019-07-08T16:14:33Z [rpc one] nFileVersion = 189900
2019-07-08T16:14:33Z [rpc one] Keys: 2001 plaintext, 0 encrypted, 2001 w/ metadata, 2001 total. Unknown wallet records: 0
2019-07-08T16:14:33Z [rpc one] Wallet completed loading in              97ms
2019-07-08T16:14:33Z GUI: TransactionTablePriv::refreshWallet
2019-07-08T16:14:33Z GUI: QMetaMethod::invoke: Unable to handle unregistered datatype 'WalletModel*'

Ref: #15453, #16349

promag · 2019-07-08T16:24:57Z

@hebasto please add WalletModel* here:

bitcoin/src/qt/bitcoin.cpp

Lines 64 to 68 in 2679bb8

    
           // Declare meta types used for QMetaObject::invokeMethod 
        
           Q_DECLARE_METATYPE(bool*) 
        
           Q_DECLARE_METATYPE(CAmount) 
        
           Q_DECLARE_METATYPE(uint256)

and test again.

hebasto · 2019-07-08T16:33:08Z

@promag

please add WalletModel* ... and test again.

Done. No luck ((
Result is the same.

promag · 2019-07-08T16:44:27Z

Also here

bitcoin/src/qt/bitcoin.cpp

Line 453 in 05623c0

qRegisterMetaType< CAmount >("CAmount");

hebasto · 2019-07-08T17:00:30Z

@promag the latter is enough:

diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index 2fdbcca04..b7740d540 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -454,6 +454,7 @@ int GuiMain(int argc, char* argv[])
     qRegisterMetaType< CAmount >("CAmount");
     qRegisterMetaType< std::function<void()> >("std::function<void()>");
     qRegisterMetaType<QMessageBox::Icon>("QMessageBox::Icon");
+    qRegisterMetaType< WalletModel* >();
     /// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
     // Command-line options take precedence:
     node->setupServerArgs();

It works.

promag · 2019-07-08T17:07:21Z

Cool, I'll add it here and you can close your PR.

hebasto · 2019-07-08T17:20:01Z

@promag

Cool, I'll add it here and you can close your PR.

I will leave it as refactoring, if you don't mind.

laanwj · 2019-07-08T17:50:19Z

Nice that this helped find an actual bug!

promag · 2019-07-09T00:39:55Z

IMO all done here, thanks @hebasto.

hebasto · 2019-07-09T04:58:12Z

ACK f27bd96, I have tested the code.

laanwj · 2019-07-09T09:47:48Z

@hebasto looks like you didn't ACK the top commit but the one below it

ACK 64fee48

64fee48 qt: Assert QMetaObject::invokeMethod result (João Barbosa) f27bd96 gui: Fix missing qRegisterMetaType(WalletModel*) (João Barbosa) Pull request description: Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the `invokeMethod` overload that allows connecting to lambdas or member pointers, which are compile checked. For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5. ACKs for top commit: laanwj: ACK 64fee48 Tree-SHA512: d332e5d7eb2c7be5d3fe90e2e4ff20a67800b9664f6637c122a23647a964f7915703d3f086e2de440f695cfe14de268ff581d0092b7736e911952a4f4d248e25

… slot 6285a31 Remove redundant WalletController::addWallet slot (Hennadii Stepanov) Pull request description: ~~Fix bitcoin#15453.~~ It is fixed by bitcoin#16348 (comment) The _only_ reason of these lines on master (8c69fae) https://github.com/bitcoin/bitcoin/blob/2679bb8919b5089f8067ccfd94f766747b8df671/src/qt/walletcontroller.cpp#L121-L128 is to `Q_EMIT walletAdded(wallet_model);` in a thread-safe manner; This PR makes this in a line of code: https://github.com/bitcoin/bitcoin/blob/1b83875006749d79916af0197bed65aecdc7ff17/src/qt/walletcontroller.cpp#L121 EDITED: To establish the ownership of a new `WalletModel` object is not necessary on the master (bitcoin#16349 (comment) by **promag**). But: > it's good habit to set ownership And I agree. It is a safe practice. ACKs for top commit: promag: ACK 6285a31. jonasschnelli: utACK 6285a31 ryanofsky: utACK 6285a31. Only change since last review is rebasing and restoring a deleted comment. I do think the comments I suggested last review would be better than this one, but this is at least better than before. Tree-SHA512: 90370cb1fe853b84dd16c3781ba4f97f3f4deca56bba0203e457f37b3220fd13228cf8495fd882ff18b7c782c27544cc2e7a88aaec5b69b9ef6d8626bdaaf332

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

1828c6f refactor: Styling w/ clang-format, comment update (Hennadii Stepanov) 88a94f7 qt: Fix missing qRegisterMetaType for size_t (Hennadii Stepanov) Pull request description: On master (a7aec7a) this connection https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/rpcconsole.cpp#L587 fails due to `ClientModel::mempoolSizeChanged()` signal has unregistered parameter type `size_t`: https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/clientmodel.h#L102 More: ``` $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- -debug=qt ... (lldb) bt * thread #17, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff35fce97 libc.so.6`__GI_raise(sig=2) at raise.c:51 frame #1: 0x00007ffff35fe801 libc.so.6`__GI_abort at abort.c:79 frame #2: 0x00007ffff5901352 libQt5Core.so.5`QMessageLogger::warning(char const*, ...) const + 354 frame #3: 0x00007ffff5b216fe libQt5Core.so.5`___lldb_unnamed_symbol2329$$libQt5Core.so.5 + 334 frame #4: 0x00007ffff5b2456d libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) + 1933 frame #5: 0x000055555566872e bitcoin-qt`ClientModel::mempoolSizeChanged(this=<unavailable>, _t1=<unavailable>, _t2=<unavailable>) at moc_clientmodel.cpp:260 ... ``` `debug.log`: ``` [] GUI: QObject::connect: Cannot queue arguments of type 'size_t' (Make sure 'size_t' is registered using qRegisterMetaType().) ``` This PR fixes it. Refs: - [Qt docs: qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#qRegisterMetaType) - #16348 --- Side NOTE: Also I believe this line https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/bitcoin.cpp#L63 is redundant since long `CAmount` is a `typedef`. ACKs for top commit: laanwj: Tested ACK 1828c6f Tree-SHA512: 2c7f9fe6a5ae70f2e1dd86b07f95d4b00c85c5706a9d722f063f80beb71880d012ec46556963fb1544c2af53d006936c2f7612eae60d9193f67db62ba3d86129

70e4706 Revert "refactor: Remove never used default parameter" (Hennadii Stepanov) 219417b Revert "refactor: Simplify connection syntax" (Hennadii Stepanov) Pull request description: The code, the `bool* ret = nullptr` parameter in the `BitcoinGUI::message()` slot, removed in #17943 is not dead actually. It is used in `ThreadSafeMessageBox()` function: https://github.com/bitcoin/bitcoin/blob/a654626f076a72416a3d354218d7107571d6caaf/src/qt/bitcoingui.cpp#L1363-L1368 Now in master (a654626): ``` $ ./src/qt/bitcoin-qt -prune=-1 Error: Prune cannot be configured with a negative value. bitcoin-qt: qt/bitcoingui.cpp:1369: bool ThreadSafeMessageBox(BitcoinGUI*, const string&, const string&, unsigned int): Assertion `invoked' failed. Aborted (core dumped) ``` This PR reverts all commits of #17943 Additional notes: the bug was missed due to dynamic function call `QMetaObject::invokeMethod()` which cannot be checked at compile time. See #16348 for more discussion. Sorry for introducing a bug. ACKs for top commit: Sjors: ACK 70e4706 laanwj: ACK 70e4706 Tree-SHA512: b968a026eaa4f5f39fd36ddc715d8e233f3c6420e6580f11d4ca422a5ff5d1d9d3df9ac11b353c3d4f434d67d6a69e37d2e26b8248d72bedd14ecba0a545a327

Summary: bitcoin/bitcoin@f27bd96 --- Depends on D6180 Partial backport of Core [[bitcoin/bitcoin#16348 | PR16348]] Test Plan: ninja check Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Subscribers: jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D6181

Summary: bitcoin/bitcoin@64fee48 --- Depends on D6181 Concludes backport of Core [[bitcoin/bitcoin#16348 | PR16348]] Test Plan: ninja check ./src/qt/bitcoin-qt -regtest Move around UI make sure everything works Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D6183

1a85aa3 Remove redundant WalletController::addWallet slot (Hennadii Stepanov) Pull request description: ~~Fix #15453.~~ It is fixed by bitcoin/bitcoin#16348 (comment) The _only_ reason of these lines on master (03465aa) https://github.com/bitcoin/bitcoin/blob/8605f6a4c7cc8cdbe7eb72600b62fa18abba372e/src/qt/walletcontroller.cpp#L121-L128 is to `Q_EMIT walletAdded(wallet_model);` in a thread-safe manner; This PR makes this in a line of code: https://github.com/bitcoin/bitcoin/blob/1b83875006749d79916af0197bed65aecdc7ff17/src/qt/walletcontroller.cpp#L121 EDITED: To establish the ownership of a new `WalletModel` object is not necessary on the master (bitcoin/bitcoin#16349 (comment) by **promag**). But: > it's good habit to set ownership And I agree. It is a safe practice. ACKs for top commit: promag: ACK 1a85aa3. jonasschnelli: utACK 1a85aa3 ryanofsky: utACK 1a85aa3. Only change since last review is rebasing and restoring a deleted comment. I do think the comments I suggested last review would be better than this one, but this is at least better than before. Tree-SHA512: 90370cb1fe853b84dd16c3781ba4f97f3f4deca56bba0203e457f37b3220fd13228cf8495fd882ff18b7c782c27544cc2e7a88aaec5b69b9ef6d8626bdaaf332

64fee48 qt: Assert QMetaObject::invokeMethod result (João Barbosa) f27bd96 gui: Fix missing qRegisterMetaType(WalletModel*) (João Barbosa) Pull request description: Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the `invokeMethod` overload that allows connecting to lambdas or member pointers, which are compile checked. For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5. ACKs for top commit: laanwj: ACK 64fee48 Tree-SHA512: d332e5d7eb2c7be5d3fe90e2e4ff20a67800b9664f6637c122a23647a964f7915703d3f086e2de440f695cfe14de268ff581d0092b7736e911952a4f4d248e25

1828c6f refactor: Styling w/ clang-format, comment update (Hennadii Stepanov) 88a94f7 qt: Fix missing qRegisterMetaType for size_t (Hennadii Stepanov) Pull request description: On master (a7aec7a) this connection https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/rpcconsole.cpp#L587 fails due to `ClientModel::mempoolSizeChanged()` signal has unregistered parameter type `size_t`: https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/clientmodel.h#L102 More: ``` $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- -debug=qt ... (lldb) bt * thread #17, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff35fce97 libc.so.6`__GI_raise(sig=2) at raise.c:51 frame #1: 0x00007ffff35fe801 libc.so.6`__GI_abort at abort.c:79 frame #2: 0x00007ffff5901352 libQt5Core.so.5`QMessageLogger::warning(char const*, ...) const + 354 frame #3: 0x00007ffff5b216fe libQt5Core.so.5`___lldb_unnamed_symbol2329$$libQt5Core.so.5 + 334 frame #4: 0x00007ffff5b2456d libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) + 1933 frame #5: 0x000055555566872e bitcoin-qt`ClientModel::mempoolSizeChanged(this=<unavailable>, _t1=<unavailable>, _t2=<unavailable>) at moc_clientmodel.cpp:260 ... ``` `debug.log`: ``` [] GUI: QObject::connect: Cannot queue arguments of type 'size_t' (Make sure 'size_t' is registered using qRegisterMetaType().) ``` This PR fixes it. Refs: - [Qt docs: qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#qRegisterMetaType) - bitcoin#16348 --- Side NOTE: Also I believe this line https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/bitcoin.cpp#L63 is redundant since long `CAmount` is a `typedef`. ACKs for top commit: laanwj: Tested ACK 1828c6f Tree-SHA512: 2c7f9fe6a5ae70f2e1dd86b07f95d4b00c85c5706a9d722f063f80beb71880d012ec46556963fb1544c2af53d006936c2f7612eae60d9193f67db62ba3d86129

… slot 6285a31 Remove redundant WalletController::addWallet slot (Hennadii Stepanov) Pull request description: ~~Fix bitcoin#15453.~~ It is fixed by bitcoin#16348 (comment) The _only_ reason of these lines on master (8c69fae) https://github.com/bitcoin/bitcoin/blob/2679bb8919b5089f8067ccfd94f766747b8df671/src/qt/walletcontroller.cpp#L121-L128 is to `Q_EMIT walletAdded(wallet_model);` in a thread-safe manner; This PR makes this in a line of code: https://github.com/bitcoin/bitcoin/blob/1b83875006749d79916af0197bed65aecdc7ff17/src/qt/walletcontroller.cpp#L121 EDITED: To establish the ownership of a new `WalletModel` object is not necessary on the master (bitcoin#16349 (comment) by **promag**). But: > it's good habit to set ownership And I agree. It is a safe practice. ACKs for top commit: promag: ACK 6285a31. jonasschnelli: utACK 6285a31 ryanofsky: utACK 6285a31. Only change since last review is rebasing and restoring a deleted comment. I do think the comments I suggested last review would be better than this one, but this is at least better than before. Tree-SHA512: 90370cb1fe853b84dd16c3781ba4f97f3f4deca56bba0203e457f37b3220fd13228cf8495fd882ff18b7c782c27544cc2e7a88aaec5b69b9ef6d8626bdaaf332

promag force-pushed the 2019-07-assert-invoke-method branch from b63de69 to 2fd7d0b Compare July 6, 2019 16:57

fanquake added the GUI label Jul 6, 2019

maflcko added the Refactoring label Jul 8, 2019

hebasto mentioned this pull request Jul 8, 2019

qt: Remove redundant WalletController::addWallet slot #16349

Merged

promag added 2 commits July 8, 2019 21:30

gui: Fix missing qRegisterMetaType(WalletModel*)

f27bd96

qt: Assert QMetaObject::invokeMethod result

64fee48

promag force-pushed the 2019-07-assert-invoke-method branch from 2fd7d0b to 64fee48 Compare July 8, 2019 20:30

laanwj merged commit 64fee48 into bitcoin:master Jul 9, 2019

fanquake removed the Needs backport label Jul 9, 2019

maflcko removed the Refactoring label Jul 9, 2019

promag mentioned this pull request Jul 13, 2019

Starting bitcoin-qt with -nowallet and then opening a wallet does not show the wallet #15453

Closed

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Aug 23, 2019

gui: Fix missing qRegisterMetaType(WalletModel*)

afc969c

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Aug 23, 2019

qt: Assert QMetaObject::invokeMethod result

b744f25

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

gui: Fix missing qRegisterMetaType(WalletModel*)

2da5dbd

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

qt: Assert QMetaObject::invokeMethod result

91f1510

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

gui: Fix missing qRegisterMetaType(WalletModel*)

97a0c7b

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

qt: Assert QMetaObject::invokeMethod result

17df7b1

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

gui: Fix missing qRegisterMetaType(WalletModel*)

9f4dbfb

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

qt: Assert QMetaObject::invokeMethod result

50d1d4e

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

gui: Fix missing qRegisterMetaType(WalletModel*)

b5857a4

Github-Pull: bitcoin#16348 Rebased-From: f27bd96

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Aug 24, 2019

qt: Assert QMetaObject::invokeMethod result

63fd041

Github-Pull: bitcoin#16348 Rebased-From: 64fee48

hebasto mentioned this pull request Nov 9, 2019

qt: Fix missing qRegisterMetaType for size_t #17427

Merged

This was referenced Jan 18, 2020

qt, refactor: Remove never used default parameter #17943

Merged

qt: Revert changes of pr17943 #17965

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

qt: Assert QMetaObject::invokeMethod result #16348

qt: Assert QMetaObject::invokeMethod result #16348

Uh oh!

Conversation

promag commented Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Jul 6, 2019

Uh oh!

promag commented Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Jul 7, 2019

Uh oh!

promag commented Jul 7, 2019

Uh oh!

hebasto commented Jul 7, 2019

Uh oh!

laanwj commented Jul 8, 2019

Uh oh!

promag commented Jul 8, 2019

Uh oh!

laanwj commented Jul 8, 2019

Uh oh!

hebasto commented Jul 8, 2019

Uh oh!

hebasto commented Jul 8, 2019

Uh oh!

promag commented Jul 8, 2019

Uh oh!

hebasto commented Jul 8, 2019

Uh oh!

promag commented Jul 8, 2019

Uh oh!

hebasto commented Jul 8, 2019

Uh oh!

promag commented Jul 8, 2019

Uh oh!

hebasto commented Jul 8, 2019

Uh oh!

laanwj commented Jul 8, 2019

Uh oh!

promag commented Jul 9, 2019

Uh oh!

hebasto commented Jul 9, 2019

Uh oh!

laanwj commented Jul 9, 2019

Uh oh!

Uh oh!

promag commented Jul 6, 2019 •

edited

Loading

DrahtBot commented Jul 6, 2019 •

edited

Loading

promag commented Jul 6, 2019 •

edited

Loading