"PSBT Operations" dialog #18027

gwillen · 2020-01-30T07:16:37Z

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' #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.

Sjors

Concept ACK

Code at b58e6f7 looks mostly alright, but Travis is very sad.

The first time I compiled this on macOS I get a cryptic error when I select the menu item:

2020-01-30 11:09:43.908 bitcoin-qt[619:453974] +[NSXPCSharedListener endpointForReply:withListenerName:]: an error occurred while attempting to obtain endpoint for listener 'com.apple.view-bridge': Connection interrupted

It worked fine the second time, so perhaps just some unrelated issue due to earlier code I compiled.

Ideally disable the sign button if there's nothing we can sign.

display status information is maybe unusual (a status bar, rather than messageboxes.)

I like it.

Currently this only changes the Load PSBT flow. Do you also want to apply this dialog to the send / bump fee flow? I'm fine with doing that later too.

You could also add a menu option to open a PSBT that's currently on the clipboard (e.g. handy when you need to save it as binary).

Sjors · 2020-01-30T10:08:05Z

src/qt/bitcoingui.cpp

@@ -317,6 +317,8 @@ void BitcoinGUI::createActions()
    signMessageAction->setStatusTip(tr("Sign messages with your Bitcoin addresses to prove you own them"));
    verifyMessageAction = new QAction(tr("&Verify message..."), this);
    verifyMessageAction->setStatusTip(tr("Verify messages to ensure they were signed with specified Bitcoin addresses"));
+    m_load_psbt_action = new QAction(tr("Load PSBT..."), this);


You should pick a letter to turn into the keyboard shortcut (works on some operating systems, not macOS). E.g. "&Load PSBT..." if L` isn't taken.

Just went with L -- it's hard for me to check what's going on here since I'm on a Mac, but I don't see it as being taken. (I didn't add a shortcut for loading from clipboard, since L is taken, C is taken, I could use P for PSBT but it seems kind of unnatural.

Sjors · 2020-01-30T10:09:21Z

src/qt/psbtoperationsdialog.h

@@ -0,0 +1,52 @@
+// Copyright (c) 2011-2014 The Bitcoin Core developers


Nit: fix year

Sjors · 2020-01-30T10:15:09Z

src/util/error.cpp

@@ -14,7 +14,7 @@ std::string TransactionErrorString(const TransactionError err)
        case TransactionError::OK:
            return "No error";
        case TransactionError::MISSING_INPUTS:
-            return "Missing inputs";
+            return "Inputs missing or spent";


Cleaning these error messages could probably be its own commit.

Yeah, agreed, will fix.

Sjors · 2020-01-30T10:24:47Z

src/qt/psbtoperationsdialog.cpp

+
+    switch (analysis.next) {
+        case PSBTRole::UPDATER: {
+            showStatus(tr("Transaction is missing some information about inputs."), StatusLevel::WARNING);


I wonder if these strings can be moved to a place where the RPC can also use them.

That would make sense actually, yeah. I could move them to wherever AnalyzePSBT is. What would the impact of this be on translation issues? I know that tr() is only used for GUI strings, and _() is used for non-GUI strings -- is there precedent for shared strings?

I'm not sure actually. @laanwj, @hebasto?

@gwillen

I know that tr() is only used for GUI strings, and _() is used for non-GUI strings -- is there precedent for shared strings?

Actually, bilingual_str type from util/translation.h supports both original and translated strings. More details are available in #16362 and #16224.

You could use something like this:

#include <util/translation.h> bilingual_str bs = _("Transaction is missing some information about inputs."); showStatus(bs.translated, StatusLevel::WARNING); LogPrintf(bs.original);

Sjors · 2020-01-30T10:26:25Z

src/qt/psbtoperationsdialog.cpp

+    return tx_description.toStdString();
+}
+
+void PSBTOperationsDialog::showStatus(const QString &msg, StatusLevel level) {


Should we create a OperationsDialog class to make this stuff reusable? (can wait of course)

I'm happy to go either way, are there other existing dialogs we'd immediately want to adopt it for? I do like it better than messageboxes, I tried to make it as drop-in as possible a replacement for them.

Sjors · 2020-01-30T10:33:14Z

src/qt/psbtoperationsdialog.cpp

+
+#include <iostream>
+
+size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction &psbt) {


Maybe move this to psbt.h?

Can do, was dithering about what to do with it.

DrahtBot · 2020-01-30T11:52:33Z

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

Conflicts

No conflicts as of last run.

gwillen · 2020-01-30T19:14:18Z

Ideally disable the sign button if there's nothing we can sign.

Can I determine that without actually trying to sign? I was assuming that (1) I cannot, and (2) I shouldn't try to actually sign for real just to see if it works. (Or do you mean just disable the button at the point where we are asked to sign, try, and fail?)

Currently this only changes the Load PSBT flow. Do you also want to apply this dialog to the send / bump fee flow? I'm fine with doing that later too.

I'd rather do that separately, I am trying really hard to cure myself of the disease of biting off way more than I can reasonably do in one go.

You could also add a menu option to open a PSBT that's currently on the clipboard (e.g. handy when you need to save it as binary).

This I'd be happy to add, since it's a very small change. Just another item below "Load PSBT...", perhaps "Load PSBT from clipboard..."?

Sjors · 2020-01-30T19:17:22Z

I indeed wouldn't try actually signing. But you e.g. tell if a wallet is watch-only, and the new ScriptPubKeyManager's can tell if they can sign for things.

gwillen · 2020-01-30T19:17:45Z

Aha, and the Travis failure is the same issue we were talking about yesterday in #bitcoin-core-dev, I got scooped by #17261 and need to rebase again and fix errors, will do. EDIT: Oh, I think actually the Travis failure is just because master was already failing Travis at the moment I made the PR. So I think it will pass as soon as I push again.

gwillen · 2020-02-01T00:36:53Z

Ok, I'm adding a check so that we do not enable the sign button (and we show a helpful note) if the current wallet is privateKeysDisabled().

I would like to use ScriptPubKeyMan to get a better verdict, but it doesn't seem like the functionality I need is quite there. It looks like what I want is something like CWallet::GetSigningProvider, which calls CanProvide on each ScriptPubKeyMan in the wallet; except that the implementation of LegacyScriptPubKeyMan::CanProvide looks like it can return true in cases where we can't actually sign, but "We can still provide some stuff if we have the script". So I can't tell if there's a call I can make that just answers the question "do we believe that this wallet can sign", without myself looping over all our ScriptPubKeyMans, and trying ProduceSignature with each with a DUMMY_SIGNATURE_CREATOR. Obviously if this is true, a better approach would be to enhance the CWallet/ScriptPubKeyMan interface to provide this, but I think that's best left for another PR. (Or if this is already provided and I'm not seeing how, please fill me in.)

gwillen · 2020-02-01T00:48:01Z

Actually, here's an approach that seems like it should work -- FillPSBT already does basically all this work for us, since it does all the signing using a HidingSigningProvider, and allows us to tell it not to sign. So if I correctly understand the behavior of HidingSigningProvider, I think this should already tell us how many inputs we could sign, it just doesn't expose the result. If that seems correct to you, I can expose that from FillPSBT, and determine that way where there's anything for us to do here. @Sjors do you know whether this sounds correct?

EDIT: No, this does not work. LegacyScriptPubKeyMan::CanProvide is playing some clever games with ProduceSignature in order to answer the "could we sign" question, and I don't want to duplicate those clever games into the PSBT logic.

SON OF EDIT: Ok, a slightly different method does work -- in FillPSBT we can effectively check whether keys are available by whether pwallet->GetSigningProvider returns anything. This ultimately calls LegacyScriptPubKeyMan::CanProvide, which has the apparent issue I described above -- it says it returns true if we don't have keys, but we do have the script -- but I don't understand exactly what that means to try to produce an example that would confuse it.

I do at least have something that works on the obvious cases now.

gwillen · 2020-02-01T02:58:06Z

Updated:

Disable sign button unless !privateKeysDisabled() and GetSigningProvider() != nullptr (I'm not sure how perfect a proxy that is for "we can sign" but it worked on my positive and negative test cases.)
Added "load from clipboard" option.
Added shortcut key in menu.
Separated error message fixes into their own commit.
Moved CountPSBTUnsignedInputs into psbt.h.

Did not change:

Send/bumpfee flow (would rather save for another PR)
Factoring out PSBT completeness description strings (not a bad idea, but could someone advise me on how that interacts with translation?)
Factoring out "OperationsDialog" (would probably rather save for another PR, and would like to understand where else we might use it first.)

Other notes:

Regarding the transient error Sjors got when clicking the menu item, all I can find when googling is that it's some transient issue that shows up for some people on OS X Catalina in multiple apps, with no obvious cause. I have no real insight there.

Please take another look?

promag · 2020-02-03T00:51:27Z

Concept ACK.

gwillen · 2020-02-08T00:27:14Z

Speculatively attempting to fix the Windows-only fails without a Windows machine to test on. It looks like MinGW defines "ERROR" as a macro, so I can't give an enum constant that name. And it looks like the Visual Studio project file for building Bitcoin-QT is not autogenerated from the Makefiles, but has to be manually updated, so I've attempted to update it.

In FillPSBT, optionally report the number of inputs we successfully signed, as an out parameter. If "sign" is false, instead report the number of inputs for which GetSigningProvider does not return nullptr. (This is a potentially overbroad estimate of inputs we could sign.) Github-Pull: bitcoin#18027 Rebased-From: 665c8e6

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. Github-Pull: bitcoin#18027 Rebased-From: 3a9939c

Github-Pull: bitcoin#18027 Rebased-From: 3e7ca1e

Sjors

tACK 3e7ca1e (conditional on original PR being merged)

Would be nice to add change detection in the PSBT summary, but it can wait for a followup.

Sjors · 2020-02-10T15:20:53Z

src/qt/psbtoperationsdialog.cpp

+            showStatus(tr("Transaction is missing some information about inputs."), StatusLevel::WARN);
+            break;
+        }
+        case PSBTRole::SIGNER: {


When all inputs have "next": "finalizer" the transaction itself may still be "next": "signer". This seems to be a bug (@achow101 or feature?) in analyzepsbt. If it's a bug, we can ignore it for this PR, if it's a feature, we should handle it, though imo it can wait for a followup.

I am going to assume this is a bug, so I'm happy to push to a followup. I have been tempted to try to rewrite the flow of AnalyzePSBT, with an eye to simplifying it and making its correctness easier to check.

Sounds like a bug

laanwj · 2020-02-10T16:18:38Z

Concept ACK!

Sjors · 2020-02-12T18:34:31Z

Mmm, it's misinterpreting a 2-of-3 multisig with two signed outputs. Probably related to my earlier comment. This is more annoying though, because it means I can't broadcast without the third signature.

gwillen · 2020-02-12T18:43:39Z

@Sjors are you able to supply me with an exact process to reproduce this? (Or an exact description -- is this a 2-of-3 using keys A, B, C, where both A and B have already signed, and you have C in your wallet, so it should be signable?)

I don't think I'm relying on analyzepsbt for this determination -- I will have to look back at the code, but I should be saying that we can sign as long as GetSigningProvider does not return nullptr. So this might just be a simple bug, or I might have misunderstood something fundamental about when we can and can't sign, but either way I'll take a look, thanks!

Sjors · 2020-02-12T19:01:53Z

It's a watch-only wallet with a 2-of-3 multisig (three hardware wallets). Two of them signed.

You probably want to call finalize on a copy of the PSBT just to see if it's complete.

gwillen · 2020-02-12T19:04:44Z

What is it misinterpreting? That is, what do you expect it to say? (If it's a watch-only wallet, then "this wallet does not have the right keys" should really say "this wallet cannot make signatures", because it's supposed to check privateKeysDisabled() first.)

Sjors · 2020-02-12T19:05:37Z

The Broadcast Transaction button should not be disabled. I can finalize it with the RPC and testmempoolaccept is happy. The rest of the info is fine I suppose.

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.

gwillen · 2020-06-19T09:24:54Z

It seems like @jb55's issue is consistent with FillPSBT not finalizing the transaction like I expected that it would, variably depending on which wallet is loaded. (@jb55, is one or both of the wallets a descriptor wallet by any chance?) FinalizePSBT calls SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, SIGHASH_ALL); on every input, whereas FillPSBT seems to bottom out in calls to SignPSBTInput(HidingSigningProvider([some signing provider], !sign, !bip32derivs), psbtx, i, sighash_type);. I would have hoped those would ultimately do the same thing, but this code is twisty, dark and deep... (And I'm nervous about the way that the ScriptPubKeyMan interface results in a somewhat complex web of signing logic getting semi-duplicated across each SPKM implementation. The wallet signing logic was just about beyond my comprehension already, before the addition of another layer.)

There are really two issues: (1) we're trying to finalize the transaction when we load it, so that we know whether it's ready or not, but failing. (2) our two information sources are not in sync (one of them we are calculating directly, the other we are getting from AnalyzePSBT, which appears to do its own finalization when checking if all signatures are present.)

I think I called FillPSBT because I wanted to know whether it thinks the PSBT is complete, and I assumed it would finalize in the process (and I think this might have been true before the SPKM refactor, though I'm not really very sure.)

So I think the fix to (1) is for me to add a call to FinalizePSBT in openWithPSBT before I call fillPSBT, which I have just done. And as for (2) I think it's okay to ignore it for now (it's not ideal that we can theoretically display conflicting information like this, but it's purely a display issue and shouldn't happen unless something else is buggy.)

The fix is harmless even if I'm wrong, so I think it's okay to call it good at this point. But I would still really really like a repro test case to play with, @jb55. (If I have correctly deduced fillPSBT's behavior, I find it surprising.)

Can we officially declare this PR done and ready (and pray that this is the last time it will need a rebase?) :-)

instagibbs · 2020-06-19T13:31:07Z

concept ACK fix, going to test again

instagibbs · 2020-06-19T13:54:21Z

tested ACK 931dd47

Tested across multiple Core wallets and HWI

Sjors · 2020-06-19T15:14:37Z

re-tACK 931dd47

jonatack · 2020-06-19T19:13:32Z

Concept ACK

achow101

ACK 931dd47

achow101 · 2020-06-19T18:43:14Z

src/wallet/scriptpubkeyman.cpp

@@ -617,6 +620,14 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb
        SignatureData sigdata;
        input.FillSignatureData(sigdata);
        SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, sighash_type);
+
+        bool signed_one = PSBTInputSigned(input);
+        if (n_signed && (signed_one || !sign)) {


The intent is to indicate whether signing happened, or whether we could sign. But this is really checking whether we could finalize, which may include signing. For instance, this wouldn't work if the input were multisig.

But that might be ok for this PR.

To reduce code duplication, maybe this should go inside SignPSBTInput?

I'm very interested in a conversation about refactoring some of this stuff. Ok to save for another PR?

(I am interested both in (1) reducing code duplication and (2) working out more precisely whether we can sign something, which I found to be very difficult to actually compute, and I definitely recognize this as a conservative approximation.)

Ok to save for another PR?

Yep

jb55 · 2020-06-20T04:45:18Z

ACK 931dd47

meshcollider · 2020-06-21T10:31:19Z

Concept ACK, glanced at the code, looks RTM

jonatack

Light code review and partially tested ACK 931dd47

Good to see this move forward and looking forward to continued progress as described in #18027 (comment). A couple of nits for the next PR follow, no need to respond to them.

jonatack · 2020-06-22T09:25:19Z

src/wallet/scriptpubkeyman.cpp

+        bool signed_one = PSBTInputSigned(input);
+        if (n_signed && (signed_one || !sign)) {
+            // If sign is false, we assume that we _could_ sign if we get here. This
+            // will never have false negatives; it is hard to tell under what i


nit: s/what i/what/ here and at line 2136 below

jonatack · 2020-06-22T09:34:54Z

src/qt/walletview.cpp

@@ -11,6 +11,7 @@
 #include <qt/askpassphrasedialog.h>
 #include <qt/clientmodel.h>
 #include <qt/guiutil.h>
+#include <qt/psbtoperationsdialog.h>


….cpp 4f9d9ef qt: Remove needless headers (Hennadii Stepanov) Pull request description: No symbols from the removed headers are used in the `qt/walletview.cpp`. This is a small followup of #18027. Top commit has no ACKs. Tree-SHA512: 986ed5c8f3bac4c0053736ce84d738f8593d3dbf713109af3cb9b7051cd838f23152a39bb3c1e9694a993c4e7accf14e94e5beff5e7881155638cd44fbf7f46f

… checking for completeness Github-Pull: bitcoin#18027 Rebased-From: a6cb0b0

….cpp 4f9d9ef qt: Remove needless headers (Hennadii Stepanov) Pull request description: No symbols from the removed headers are used in the `qt/walletview.cpp`. This is a small followup of bitcoin#18027. Top commit has no ACKs. Tree-SHA512: 986ed5c8f3bac4c0053736ce84d738f8593d3dbf713109af3cb9b7051cd838f23152a39bb3c1e9694a993c4e7accf14e94e5beff5e7881155638cd44fbf7f46f

fanquake added the GUI label Jan 30, 2020

Sjors reviewed Jan 30, 2020

View reviewed changes

gwillen force-pushed the feature-psbt-ops-dialog branch from b58e6f7 to 078e3ca Compare February 1, 2020 02:46

gwillen force-pushed the feature-psbt-ops-dialog branch from 078e3ca to 3e7ca1e Compare February 8, 2020 00:18

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Feb 9, 2020

[gui] Load PSBT from clipboard

199b587

Github-Pull: bitcoin#18027 Rebased-From: 3e7ca1e

Sjors approved these changes Feb 10, 2020

View reviewed changes

Sjors mentioned this pull request Feb 12, 2020

Descriptor wallet + PSBT GUI support Sjors/bitcoin#13

Closed

gwillen force-pushed the feature-psbt-ops-dialog branch from 2a617cd to ca2144f Compare June 19, 2020 07:09

DrahtBot removed the Needs rebase label Jun 19, 2020

gwillen added 3 commits June 19, 2020 02:20

[gui] PSBT Operations Dialog (sign & broadcast)

a6cb0b0

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.

[gui] Load PSBT from clipboard

11a0ffb

Make lint-spelling.py happy

931dd47

gwillen force-pushed the feature-psbt-ops-dialog branch from ca2144f to 931dd47 Compare June 19, 2020 09:24

achow101 reviewed Jun 19, 2020

View reviewed changes

DrahtBot mentioned this pull request Jun 20, 2020

validation: re-delegate absurd fee checking from mempool to clients #19339

Merged

meshcollider merged commit c273308 into bitcoin:master Jun 21, 2020

jonatack reviewed Jun 22, 2020

View reviewed changes

This was referenced Jun 23, 2020

Remove needless headers from qt/walletview.cpp bitcoin-core/gui#11

Merged

refactor: Replace RecursiveMutex with Mutex in CTxMemPool #19306

Closed

ryanofsky mentioned this pull request Jul 7, 2020

Multiprocess bitcoin #10102

Draft

hebasto mentioned this pull request Jul 10, 2020

qt, refactor: Make BitcoinUnits::Unit a scoped enum #17877

Closed

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Aug 15, 2020

GUI/PSBT: Make sure all existing signatures are fully combined before…

165a270

… checking for completeness Github-Pull: bitcoin#18027 Rebased-From: a6cb0b0

Rspigler mentioned this pull request Oct 27, 2020

Proposed PSBT sign/broadcast flow #17717

Closed

achow101 mentioned this pull request Dec 3, 2020

Partially Signed Bitcoin Transaction (PSBT) GUI for Bitcoin Core Request #20515

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

		@@ -0,0 +1,52 @@
		// Copyright (c) 2011-2014 The Bitcoin Core developers


		#include <iostream>

		size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction &psbt) {

"PSBT Operations" dialog #18027

"PSBT Operations" dialog #18027

Uh oh!

Conversation

gwillen commented Jan 30, 2020 • edited by meshcollider Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jan 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

gwillen commented Jan 30, 2020

Uh oh!

Sjors commented Jan 30, 2020

Uh oh!

gwillen commented Jan 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gwillen commented Feb 1, 2020

Uh oh!

gwillen commented Feb 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gwillen commented Feb 1, 2020

Uh oh!

promag commented Feb 3, 2020

Uh oh!

gwillen commented Feb 8, 2020

Uh oh!

Sjors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Feb 10, 2020

Uh oh!

Sjors commented Feb 12, 2020

Uh oh!

gwillen commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

gwillen commented Jan 30, 2020 •

edited by meshcollider

Loading

Sjors left a comment •

edited

Loading

DrahtBot commented Jan 30, 2020 •

edited

Loading

gwillen commented Jan 30, 2020 •

edited

Loading

gwillen commented Feb 1, 2020 •

edited

Loading

gwillen commented Feb 12, 2020 •

edited

Loading

Sjors commented Feb 12, 2020 •

edited

Loading

Sjors commented Feb 12, 2020 •

edited

Loading

gwillen commented Jun 19, 2020 •

edited

Loading

instagibbs commented Jun 19, 2020 •

edited

Loading