wallet: Properly rebroadcast unconfirmed transaction chains #25768

achow101 · 2022-08-02T00:45:36Z

Currently ResendWalletTransactions (used for normal rebroadcasts) will attempt to rebroadcast all of the transactions in the wallet in the order they are stored in mapWallet. This ends up being random as mapWallet is a std::unordered_map. However ReacceptWalletTransactions (used for adding to the mempool on loading) first sorts the txs by wallet insertion order, then submits them. The result is that ResendWalletTranactions will fail to rebroadcast child transactions if their txids happen to be lexicographically less than their parent's txid. This PR resolves this issue by combining ReacceptWalletTransactions and ResendWalletTransactions into a new ResubmitWalletTransactions so that the iteration code and basic checks are shared.

A test has also been added that checks that such transaction chains are rebroadcast correctly.

DrahtBot · 2022-08-02T02:59:34Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#24683 (Add and use ForEachWallet by promag)
#20892 (tests: Run both descriptor and legacy tests within a single test invocation by achow101)

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.

glozow

Concept ACK

test/functional/wallet_resendwallettransactions.py

src/wallet/wallet.cpp

kevkevinpal · 2022-08-17T01:22:40Z

Concept ACK
was able to run the functional test on my local and it ran fine, still want to test on regtest but still new to the project so still learning

src/wallet/wallet.cpp

Riahiamirreza · 2022-08-21T16:27:20Z

Would you explain what is the difference between ResendWalletTransactions and ReacceptWalletTransactions? As you said ResendWalletTransactions is used for normal rebroadcasting and ReacceptWalletTransactions is used for rebroadcasting on loading. I'm not sure what do you mean by "loading", you mean something like reindex or reorg?

achow101 · 2022-08-21T16:36:54Z

I'm not sure what do you mean by "loading", you mean something like reindex or reorg?

"Loading" as in when the wallet is read from disk and loaded into memory.

Riahiamirreza · 2022-08-21T16:55:12Z

So what is the difference between rebroadcasting when loading and in normal cases?

achow101 · 2022-08-21T17:04:32Z

There are differences in which transactions will be considered for rebroadcasting. On load, all non-abandoned transactions will be rebroadcast. The periodic rebroadcast will only rebroadcast those that are more than 5 minutes older than the most recent block.

test/functional/wallet_resendwallettransactions.py

src/wallet/wallet.cpp

ghost · 2022-08-24T09:03:44Z

This PR resolves this issue by refactoring ReacceptWalletTransactions sorting code into a separate function, then using that function in both ReacceptWalletTransactions and ResendWalletTransactions so that both functions will sort the transactions in the same order before processing for rebroadcast.

Is it possible to shuffle transactions before SubmitTxMemoryPoolAndRelay rebroadcasting using std::shuffle? Since only wallet txs are rebroadcasted which is a separate privacy issue, we could at least hide the order in which they were inserted in wallet db.

rajarshimaitra

Concept + tACK e40ddf3

Verified that the tests are intermittently failing.
Failure occurring at debug log assertion ResendWalletTransactions: resubmit 2 unconfirmed transactions

Below are just one nit comment and a conceptual question.

test/functional/wallet_resendwallettransactions.py

achow101 · 2022-08-24T15:54:04Z

Is it possible to shuffle transactions before SubmitTxMemoryPoolAndRelay rebroadcasting using std::shuffle? Since only wallet txs are rebroadcasted which is a separate privacy issue, we could at least hide the order in which they were inserted in wallet db.

No. Shuffling is the reason we have this issue in the first place (there is no order in an unsorted map so iterating it is basically the same as iterating a shuffled list). I don't think the insertion order is a privacy leak since it's the same order as when the transactions were originally broadcast which is public information to anyone that's connected to the network. Additionally, this is not a unilateral broadcast - it is up to the mempool relay code to determine when, how, and which transactions are actually rebroadcast. Only transactions that are not already in the mempool will be rebroadcast.

ghost · 2022-08-24T16:22:50Z

No. Shuffling is the reason we have this issue in the first place (there is no order in an unsorted map so iterating it is basically the same as iterating a shuffled list).

I was suggesting to shuffle the sorted list before rebroadcast. So we have all the transactions we need but do not relay in that order.

I don't think the insertion order is a privacy leak since it's the same order as when the transactions were originally broadcast which is public information to anyone that's connected to the network.

In that case, feel free to ignore my suggestion.

ghost · 2022-08-24T16:23:01Z

Concept ACK

achow101 · 2022-08-24T16:34:31Z

I was suggesting to shuffle the sorted list before rebroadcast. So we have all the transactions we need but do not relay in that order.

Shuffling after sorting would just undo the sorting. Sorting is not used for filtering; it is used because we need to have the transactions in order. Specifically parent transactions must be rebroadcast before child transactions, and that specific order can be obtained through the insertion order.

While we could do something more complicated like actually building packages so that the actually order dependent things are grouped together, and then shuffling the packages, but I don't see the benefit in doing that. That might actually harm privacy because we would be broadcasting transactions that are related to each other at the same time, but only those transactions that are related to our wallet. Since a package that the wallet builds may exclude other transactions in the package that the mempool would know about, we could essentially be telling other nodes some of the scripts that our wallet is watching for as they could then look at what the rebroadcast package has that the rest of the package in the mempool does not.

achow101 · 2022-08-24T21:52:01Z

Based on some discussions during the PR review club today, I've made several changes to this PR. The first is that ResendWalletTransaction and ReacceptWalletTransactions are now combined into a single function ResubmitWalletTranasctions. There were some subtle differences between the two functions that are being preserved by using two optional parameters. The first difference is that ReacceptWalletTransactions would not actually ask the mempool to relay the transactions (there is a parameter for relay in SubmitTxMemoryPoolAndRelay that was different in both). There is a parameter relay with a default of true to handle that. The second is that ResendWalletTransactions would look chain status (is ibd, reindexing, etc.) and transaction time in order to determine whether to try submitting a transaction, whereas ReacceptWalletTransactions would not. To match the ReaccpetWalletTransactions behavior, a force option with a default of false is added that will bypass those checks.

Additionally, nNextResend was initialized the first time ResendWalletTransactions was run. As the new ResubmitWalletTransactions is always run on start, I've removed from the test the need to force it to be run as well as the handling around not actually trying to resend during that first run (we want to do the reaccept behavior in that first run). I've also removed GetSortedTxs and have instead taken the suggestion of filtering then sorting the transactions. This is done by inserting the CWalletTxs that we want to submit into a std::set with a custom comparator that uses CWalletTx.nOrderPos to determine the sort order.

Lastly I've updated the test to reliably fail on master. I found that listreceivedbyaddress and related RPCs will output txids in the order that they are found in mapWallet. The new test will use that to check if the child is positioned before the parent, and if it is not, it will remake the child until it does.

ghost

ACK fee0dea

hernanmarino

ACK fee0dea

Now the test consistently fails on master, as it should.

LarryRuane

ACK 748d5c7

src/wallet/wallet.cpp

Both of these functions do almost the exact same thing. They can be deduplicated so that their behavior matches except for the filtering aspect. As this function will now always be called on wallet loading, nNextResend will also always be initialized, so wallet_resendwallettransactions.py is updated to account for that. This also resolves a bug where ResendWalletTransactions would fail to rebroadcast txs in insertion order thereby potentially rebroadcasting a child transaction before its parent and causing the child to not actually get rebroadcast. Also names the combined function to ResubmitWalletTransactions as the function just submits the transactions to the mempool rather than doing any sending by itself.

The test checks that parent txs are broadcast before child txs. The previous behavior is that the rebroadcasting would simply iterate mapWallet. As mapWallet is a std::unsorted_map, the child can sometimes come before the parent and thus be rebroadcast in the wrong order and fail the test.

ghost

reACK 3405f3e

naumenkogs · 2022-08-30T06:19:24Z

utACK 3405f3e

furszy

Late code review ACK 3405f3e

The only orthogonal question that arises from my side is why are we trying to resubmit txes after every import RPC command. Can anything change at the, already created, wallet txes level after an import?

furszy · 2022-08-30T12:38:18Z

src/wallet/wallet.cpp

    // During reindex, importing and IBD, old wallet transactions become
    // unconfirmed. Don't resend them as that would spam other nodes.
-    if (!chain().isReadyToBroadcast()) return;
+    // We only allow forcing mempool submission when not relaying to avoid this spam.
+    if (!force && relay && !chain().isReadyToBroadcast()) return;

    // Do this infrequently and randomly to avoid giving away
    // that these are our transactions.
-    if (GetTime() < nNextResend || !fBroadcastTransactions) return;
-    bool fFirst = (nNextResend == 0);
+    if (!force && GetTime() < nNextResend) return;


At this point of the review, this is just a minor nit but code would be a bit clearer in this way:

if (!force) { // During reindex, importing and IBD, old wallet transactions become // unconfirmed. Don't resend them as that would spam other nodes. // We only allow forcing mempool submission when not relaying to avoid this spam. if (relay && !chain().isReadyToBroadcast()) return; // Do this infrequently and randomly to avoid giving away // that these are our transactions. if (GetTime() < nNextResend) return; }

I think both these checks would be more appropriate in MaybeResendWalletTxs() since they're more scheduling-like in nature. It would also address furszy's comment them since we don't need to check for force anymore (it's always True there). If you do this, you probably also want to duplicate if (!fBroadcastTransactions) there since it's a very cheap check and we could avoid iterating over the wallets.

Unfortunately, we still need the force parameter when iterating over the transactions, otherwise ResubmitWalletTransactions()'s signature could have been simplified.

maflcko

left some minor nits, which can be addressed the next time this code is touched

maflcko · 2022-08-30T14:31:52Z

test/functional/wallet_resendwallettransactions.py

+        evict_time = block_time + 60 * 60 * DEFAULT_MEMPOOL_EXPIRY_HOURS + 5
+        node.setmocktime(evict_time)
+        indep_send = node.send(outputs=[{node.getnewaddress(): 1}], options={"inputs": [indep_utxo]})
+        node.syncwithvalidationinterfacequeue()


Can you explain why this is needed? Generally this is only needed when forcing the wallet to receive all in-flight mempool notifications. However, there is no wallet call that follows. Also, there is no m_best_block_time update.

Also, there is no m_best_block_time update.

Why not? IIRC this was done in order to update m_best_block_time so that the transaction would be properly evicted later.

The transaction is evicted from the mempool, not the wallet. (Only the wallet has an m_best_block_time)

Also, there is no new best block, so m_best_block_time won't be updated either way.

Oh, wrong line. I think this syncwithvalidationinterfacequeue is probably unneeded.

maflcko · 2022-08-30T14:35:27Z

test/functional/wallet_resendwallettransactions.py

+        block = create_block(int(node.getbestblockhash(), 16), create_coinbase(node.getblockcount() + 1), block_time)
+        block.solve()
+        node.submitblock(block.serialize().hex())
+        node.syncwithvalidationinterfacequeue()


Can you explain why this is needed? Maybe duplicate the comment "Set correct m_best_block_time" from above?

maflcko · 2022-08-30T14:39:38Z

src/wallet/wallet.cpp

+// The `force` option results in all unconfirmed transactions being submitted to
+// the mempool. This does not necessarily result in those transactions being relayed,
+// that depends on the `relay` option. Periodic rebroadcast uses the pattern
+// relay=true force=false (also the default values), while loading into the mempool


Suggested change

// relay=true force=false (also the default values), while loading into the mempool

// relay=true force=false, while loading into the mempool

maflcko · 2022-08-30T14:40:08Z

src/wallet/wallet.cpp

+            // Only rebroadcast unconfirmed txs
+            if (!wtx.isUnconfirmed()) continue;
+
+            // attempt to rebroadcast all txes more than 5 minutes older than


Suggested change

// attempt to rebroadcast all txes more than 5 minutes older than

// Attempt to rebroadcast all txes more than 5 minutes older than

Any reason that this A changed into a?

stickies-v

ACK 3405f3e

stickies-v · 2022-08-26T15:13:10Z

src/wallet/wallet.cpp

+// Resubmit transactions from the wallet to the mempool, optionally asking the
+// mempool to relay them. On startup, we will do this for all unconfirmed


nit: perhaps this first sentence should live in wallet.h?

stickies-v · 2022-09-05T11:59:26Z

src/wallet/wallet.cpp

    // During reindex, importing and IBD, old wallet transactions become
    // unconfirmed. Don't resend them as that would spam other nodes.
-    if (!chain().isReadyToBroadcast()) return;
+    // We only allow forcing mempool submission when not relaying to avoid this spam.
+    if (!force && relay && !chain().isReadyToBroadcast()) return;

    // Do this infrequently and randomly to avoid giving away
    // that these are our transactions.
-    if (GetTime() < nNextResend || !fBroadcastTransactions) return;
-    bool fFirst = (nNextResend == 0);
+    if (!force && GetTime() < nNextResend) return;


I think both these checks would be more appropriate in MaybeResendWalletTxs() since they're more scheduling-like in nature. It would also address furszy's comment them since we don't need to check for force anymore (it's always True there). If you do this, you probably also want to duplicate if (!fBroadcastTransactions) there since it's a very cheap check and we could avoid iterating over the wallets.

Unfortunately, we still need the force parameter when iterating over the transactions, otherwise ResubmitWalletTransactions()'s signature could have been simplified.

…tion ch…

See bitcoin#25768 (comment) Also fix doc typo from bitcoin#25768 (comment)

maflcko · 2022-09-20T10:09:23Z

src/wallet/rpc/backup.cpp

@@ -295,7 +295,7 @@ RPCHelpMan importaddress()
        RescanWallet(*pwallet, reserver);
        {
            LOCK(pwallet->cs_wallet);


The locks are no longer needed and can be removed

maflcko · 2022-09-20T16:40:35Z

If a user calls importaddress every 6 hours, this will block any p2p relay from ever happening?

stickies-v · 2022-09-29T17:05:37Z

If a user calls importaddress every 6 hours, this will block any p2p relay from ever happening?

I addressed that in #26205

b01682a refactor: revert m_next_resend to not be std::atomic (stickies-v) 9245f45 wallet: only update m_next_resend when actually resending (stickies-v) 7fbde8a refactor: carve out tx resend timer logic into ShouldResend (stickies-v) 01f3534 refactor: remove unused locks for ResubmitWalletTransactions (stickies-v) c6e8e11 wallet: fix capitalization in docstring (stickies-v) Pull request description: This PR addresses the outstanding comments/issues from #25768: - capitalization [typo](bitcoin/bitcoin#25768 (comment)) in docstring - remove [unused locks](bitcoin/bitcoin@01f3534) that we previously needed for `ReacceptWalletTransactions()` - before #25768, only `ResendWalletTransactions()` would reset `m_next_resend` (formerly called `nNextResend`). By unifying it with `ReacceptWalletTransactions()` into `ResubmitWalletTransactions()`, the number of callsites that would reset the `m_next_resend` timer increased - since `m_next_resend` is only used in case of `relay=true` (formerly `ResendWalletTransactions()`), this is unintuitive - it leads to [unexpected behaviour](bitcoin/bitcoin#25768 (comment)) such as transactions potentially never being rebroadcasted. - it makes the ResubmitWalletTransactions()` logic [more complicated than strictly necessary](bitcoin/bitcoin#25768 (comment)) - since #25768, we relied on an earlier call of `ResubmitWalletTransactions(relay=false, force=true)` to initialize `m_next_resend()`, I think we can more elegantly do that by just providing `m_next_resend` with a default value - just to highlight: this commit introduces behaviour change Note: the `if (!fBroadcastTransactions)` in `CWallet:ShouldResend()` is duplicated on purpose, since it potentially avoids the slightly more expensive `if (!chain().isReadyToBroadcast())` check afterwards. I don't have a strong view on it, so happy to remove that additional check to reduce the diff, too. ACKs for top commit: aureleoules: ACK b01682a achow101: ACK b01682a Tree-SHA512: ac5f1d8858f8dd736dd1480f385984d660c1916b62a42562317020e8f9fd6a30bd8f23d973d47e4c9480d744c5ba39fdbefd69568a5eb0589a8422d7e5971c1c

b01682a refactor: revert m_next_resend to not be std::atomic (stickies-v) 9245f45 wallet: only update m_next_resend when actually resending (stickies-v) 7fbde8a refactor: carve out tx resend timer logic into ShouldResend (stickies-v) 01f3534 refactor: remove unused locks for ResubmitWalletTransactions (stickies-v) c6e8e11 wallet: fix capitalization in docstring (stickies-v) Pull request description: This PR addresses the outstanding comments/issues from bitcoin#25768: - capitalization [typo](bitcoin#25768 (comment)) in docstring - remove [unused locks](bitcoin@01f3534) that we previously needed for `ReacceptWalletTransactions()` - before bitcoin#25768, only `ResendWalletTransactions()` would reset `m_next_resend` (formerly called `nNextResend`). By unifying it with `ReacceptWalletTransactions()` into `ResubmitWalletTransactions()`, the number of callsites that would reset the `m_next_resend` timer increased - since `m_next_resend` is only used in case of `relay=true` (formerly `ResendWalletTransactions()`), this is unintuitive - it leads to [unexpected behaviour](bitcoin#25768 (comment)) such as transactions potentially never being rebroadcasted. - it makes the ResubmitWalletTransactions()` logic [more complicated than strictly necessary](bitcoin#25768 (comment)) - since bitcoin#25768, we relied on an earlier call of `ResubmitWalletTransactions(relay=false, force=true)` to initialize `m_next_resend()`, I think we can more elegantly do that by just providing `m_next_resend` with a default value - just to highlight: this commit introduces behaviour change Note: the `if (!fBroadcastTransactions)` in `CWallet:ShouldResend()` is duplicated on purpose, since it potentially avoids the slightly more expensive `if (!chain().isReadyToBroadcast())` check afterwards. I don't have a strong view on it, so happy to remove that additional check to reduce the diff, too. ACKs for top commit: aureleoules: ACK b01682a achow101: ACK b01682a Tree-SHA512: ac5f1d8858f8dd736dd1480f385984d660c1916b62a42562317020e8f9fd6a30bd8f23d973d47e4c9480d744c5ba39fdbefd69568a5eb0589a8422d7e5971c1c

See bitcoin/bitcoin#25768 (comment) Also fix doc typo from bitcoin/bitcoin#25768 (comment)

DrahtBot added the Wallet label Aug 2, 2022

glozow reviewed Aug 2, 2022

View reviewed changes

achow101 force-pushed the unify-resend-reaccept branch from cb4131c to b11ccea Compare August 2, 2022 21:12

DrahtBot mentioned this pull request Aug 4, 2022

tests: Run both descriptor and legacy tests within a single test invocation #20892

Closed

glozow reviewed Aug 8, 2022

View reviewed changes

test/functional/wallet_resendwallettransactions.py Show resolved Hide resolved

kevkevinpal reviewed Aug 17, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

achow101 force-pushed the unify-resend-reaccept branch 2 times, most recently from 901b864 to e40ddf3 Compare August 18, 2022 16:26

Riahiamirreza reviewed Aug 20, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

glozow reviewed Aug 22, 2022

View reviewed changes

test/functional/wallet_resendwallettransactions.py Outdated Show resolved Hide resolved

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Aug 24, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Aug 24, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

stickies-v reviewed Aug 24, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

rajarshimaitra reviewed Aug 24, 2022

View reviewed changes

test/functional/wallet_resendwallettransactions.py Outdated Show resolved Hide resolved

test/functional/wallet_resendwallettransactions.py Outdated Show resolved Hide resolved

achow101 force-pushed the unify-resend-reaccept branch from e40ddf3 to 85be5fb Compare August 24, 2022 21:40

achow101 force-pushed the unify-resend-reaccept branch from 85be5fb to fee0dea Compare August 24, 2022 21:55

ghost approved these changes Aug 24, 2022

View reviewed changes

hernanmarino approved these changes Aug 25, 2022

View reviewed changes

LarryRuane approved these changes Aug 29, 2022

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

achow101 added 2 commits August 29, 2022 12:38

achow101 force-pushed the unify-resend-reaccept branch from 748d5c7 to 3405f3e Compare August 29, 2022 16:42

ghost approved these changes Aug 29, 2022

View reviewed changes

furszy approved these changes Aug 30, 2022

View reviewed changes

maflcko reviewed Aug 30, 2022

View reviewed changes

glozow requested a review from stickies-v September 1, 2022 11:40

stickies-v reviewed Sep 5, 2022

View reviewed changes

maflcko added this to the 24.0 milestone Sep 5, 2022

glozow merged commit 5291933 into bitcoin:master Sep 5, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 5, 2022

Merge bitcoin#25768: wallet: Properly rebroadcast unconfirmed transac…

66165db

…tion ch…

bitcoin deleted a comment Sep 5, 2022

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Sep 14, 2022

test/doc: Remove unused syncwithvalidationinterfacequeue

fa44670

See bitcoin#25768 (comment) Also fix doc typo from bitcoin#25768 (comment)

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Sep 14, 2022

test/doc: Remove unused syncwithvalidationinterfacequeue

faa4916

See bitcoin#25768 (comment) Also fix doc typo from bitcoin#25768 (comment)

maflcko reviewed Sep 20, 2022

View reviewed changes

stickies-v mentioned this pull request Sep 29, 2022

wallet: #25768 follow ups #26205

Merged

janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Jan 20, 2023

test/doc: Remove unused syncwithvalidationinterfacequeue

d23ce23

See bitcoin/bitcoin#25768 (comment) Also fix doc typo from bitcoin/bitcoin#25768 (comment)

bitcoin locked and limited conversation to collaborators Nov 4, 2023

	// relay=true force=false (also the default values), while loading into the mempool
	// relay=true force=false, while loading into the mempool

	// attempt to rebroadcast all txes more than 5 minutes older than
	// Attempt to rebroadcast all txes more than 5 minutes older than

		// Resubmit transactions from the wallet to the mempool, optionally asking the
		// mempool to relay them. On startup, we will do this for all unconfirmed

wallet: Properly rebroadcast unconfirmed transaction chains #25768

wallet: Properly rebroadcast unconfirmed transaction chains #25768

Uh oh!

Conversation

achow101 commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

glozow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kevkevinpal commented Aug 17, 2022

Uh oh!

Uh oh!

Riahiamirreza commented Aug 21, 2022

Uh oh!

achow101 commented Aug 21, 2022

Uh oh!

Riahiamirreza commented Aug 21, 2022

Uh oh!

achow101 commented Aug 21, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Aug 24, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajarshimaitra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

achow101 commented Aug 24, 2022

Uh oh!

ghost commented Aug 24, 2022

Uh oh!

ghost commented Aug 24, 2022

Uh oh!

achow101 commented Aug 24, 2022

Uh oh!

achow101 commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

LarryRuane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

naumenkogs commented Aug 30, 2022

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stickies-v Sep 5, 2022 • 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!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

achow101 commented Aug 2, 2022 •

edited

Loading

DrahtBot commented Aug 2, 2022 •

edited

Loading

ghost commented Aug 24, 2022 •

edited by ghost

Loading

achow101 commented Aug 24, 2022 •

edited

Loading

stickies-v Sep 5, 2022 •

edited

Loading

stickies-v Sep 5, 2022 •

edited

Loading