p2p: Clarify control flow in ProcessMessage #13946

maflcko · 2018-08-12T11:59:30Z

ProcessMessage is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in ProcessMessage() by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach.
Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162)

This patch does exactly that.

Also note that this patch is a subset of previous approaches such as #9608 and #10145.

Review suggestion: git diff HEAD~ --function-context

practicalswift · 2018-08-12T14:45:06Z

utACK fa37c5b662f1aeeaa53e60377979156e84d550b4

Clarification is good

domob1812 · 2018-08-12T15:52:32Z

utACK fa37c5b662f1aeeaa53e60377979156e84d550b4

Seems like a reasonable clarification with minimal changes.

DrahtBot · 2018-08-12T17:03:14Z

Note to reviewers: This pull request conflicts with the following ones:

Add p2p layer encryption with ECDH/ChaCha20Poly1305 #14032 (Add p2p layer encryption with ECDH/ChaCha20Poly1305 by jonasschnelli)
Dandelion transaction relay (BIP 156) #13947 (Dandelion transaction relay (BIP 156) by MarcoFalke)

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.

sipa · 2018-08-12T17:26:57Z

Any reason not to drop all the elses in between the different options?

maflcko · 2018-08-12T19:13:48Z

Made everything early-return as requested by @sipa

Empact · 2018-08-13T01:25:20Z

utACK fa6c3de

nit: This block would be a bit easier to parse if the returns were one level up:

bitcoin/src/net_processing.cpp

Lines 1585 to 1594 in 0df7a6c

    
           { 
        
               if (pfrom->nVersion >= NO_BLOOM_VERSION) { 
        
                   LOCK(cs_main); 
        
                   Misbehaving(pfrom->GetId(), 100); 
        
                   return false; 
        
               } else { 
        
                   pfrom->fDisconnect = true; 
        
                   return false; 
        
               } 
        
           }

nit: Not necessarily to be fixed in this PR, but I noticed that due to !fImporting && !fReindex, CMPCTBLOCK, BLOCKTXN, HEADERS, and BLOCK messages will be treated with
LogPrint(BCLog::NET, "Unknown command \"%s\" from peer=%d\n" during import or reindex. The log message could be more informative.

practicalswift · 2018-08-13T07:17:21Z

utACK fa6c3de

jnewbery

utACK fa6c3de

one comment inline.

jnewbery · 2018-08-24T17:26:09Z

src/net_processing.cpp

    }

-
-    else if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing
+    if (strCommand == NetMsgType::CMPCTBLOCK && !fImporting && !fReindex) // Ignore blocks received while importing


May be out of scope for this PR, but I think it'd be better for these conditionals to be nested:

if (strCommand == NetMsgType::CMPCTBLOCK) { if (!fImporting && !fReindex) { // Ignore blocks received while importing ... } // could optionally log in an else branch here return true; }

for a couple of reasons:

it prevents compact blocks from being logged as "Unknown command" if received during import/reindex

it maintains your new control flow of each message type returning after its own code block.

Same for BLOCKTXN, HEADERS and BLOCK message code blocks below.

Indeed out of scope, as this is not refactoring, but bugfixing-like territory.

laanwj · 2018-08-25T16:18:08Z

I'm not sure this is much better with the early returns. though, this does untangle the case and makes it easier to split up the function and move to dispatching based on say, a hash table, in the future, so it's a step in the right direction.

utACK fa6c3de

fa6c3de p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as #9608 and #10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201

JeremyRubin · 2018-09-16T18:46:33Z

post-merge utack, good incremental step.

fa6c3de p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see bitcoin#9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix bitcoin#13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as bitcoin#9608 and bitcoin#10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201

fa6c3de p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see bitcoin#9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix bitcoin#13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as bitcoin#9608 and bitcoin#10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201 More of 13946

fa6c3de p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see bitcoin#9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix bitcoin#13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as bitcoin#9608 and bitcoin#10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201

* commit '351fbf65efc9459cb69a3c843cc205a8b94c95b3': (987 commits) Update release-notes Bump nMinimumChainWork and defaultAssumeValid (dashpay#3336) Update release notes Try to actually accept newly created dstx-es into masternode's mempool (dashpay#3332) Switch CLIENT_VERSION_IS_RELEASE to `true` for v0.15 (dashpay#3306) Update release notes Bring back "about" menu icon (dashpay#3329) Add pubKeyOperator to `quorum info` rpc response (dashpay#3327) Update release-notes.md Update translations 2020-02-03 (dashpay#3322) Only sync mempool from v0.15+ (proto 70216+) nodes (dashpay#3321) Fix dark text on dark background in combobox dropdowns on windows (dashpay#3315) Fix node protection logic false positives (dashpay#3314) Merge bitcoin#13162: [net] Don't incorrectly log that REJECT messages are unknown. More of 13946 Merge bitcoin#13946: p2p: Clarify control flow in ProcessMessage Add `automake` package to dash-win-signer's packages list (dashpay#3307) [Trivial] Release note update (dashpay#3308) Update release-notes.md Fix CActiveMasternodeManager::GetLocalAddress to prefer IPv4 if multiple local addresses are known (dashpay#3304) ... # Conflicts: # .gitignore # .travis.yml # COPYING # Makefile.am # README.md # ci/test_unittests.sh # configure.ac # contrib/README.md # contrib/debian/control # contrib/devtools/optimize-pngs.py # contrib/gitian-build.py # contrib/gitian-descriptors/gitian-linux.yml # contrib/gitian-descriptors/gitian-osx.yml # contrib/gitian-descriptors/gitian-win-signer.yml # contrib/gitian-descriptors/gitian-win.yml # contrib/init/README.md # contrib/seeds/README.md # contrib/verifybinaries/verify.sh # doc/README.md # doc/README_windows.txt # doc/build-generic.md # doc/build-unix.md # doc/files.md # doc/man/biblepay-cli.1 # doc/man/biblepay-qt.1 # doc/man/biblepay-tx.1 # doc/man/biblepayd.1 # doc/release-notes.md # doc/release-process.md # qa/README.md # qa/rpc-tests/autois-mempool.py # qa/rpc-tests/maxblocksinflight.py # qa/rpc-tests/multikeysporks.py # qa/rpc-tests/p2p-acceptblock.py # qa/rpc-tests/p2p-autoinstantsend.py # share/pixmaps/dash128.png # share/pixmaps/dash16.png # share/pixmaps/dash32.png # share/pixmaps/dash64.png # src/Makefile.am # src/Makefile.qt.include # src/Makefile.qttest.include # src/Makefile.test.include # src/addrman.h # src/alert.cpp # src/alert.h # src/bench/bls.cpp # src/bench/crypto_hash.cpp # src/biblepay-cli.cpp # src/biblepayd.cpp # src/blockencodings.cpp # src/bls/bls.cpp # src/bls/bls.h # src/bls/bls_worker.cpp # src/bls/bls_worker.h # src/cachemap.h # src/cachemultimap.h # src/chain.h # src/chainparams.cpp # src/checkpoints.cpp # src/clientversion.h # src/evo/cbtx.cpp # src/evo/cbtx.h # src/evo/deterministicmns.h # src/evo/evodb.cpp # src/evo/evodb.h # src/evo/providertx.cpp # src/evo/providertx.h # src/evo/simplifiedmns.h # src/evo/specialtx.cpp # src/evo/specialtx.h # src/flat-database.h # src/governance/governance-classes.cpp # src/governance/governance-exceptions.h # src/governance/governance-object.cpp # src/governance/governance-object.h # src/governance/governance-validators.cpp # src/governance/governance-validators.h # src/governance/governance-vote.h # src/governance/governance-votedb.cpp # src/governance/governance-votedb.h # src/governance/governance.cpp # src/hash.h # src/hdchain.cpp # src/hdchain.h # src/httpserver.cpp # src/init.cpp # src/instantx.cpp # src/instantx.h # src/keepass.cpp # src/keepass.h # src/llmq/quorums_blockprocessor.h # src/llmq/quorums_commitment.h # src/masternode/activemasternode.cpp # src/masternode/masternode-payments.cpp # src/masternode/masternode-sync.cpp # src/messagesigner.cpp # src/messagesigner.h # src/miner.cpp # src/net.cpp # src/net.h # src/net_processing.cpp # src/netbase.cpp # src/netfulfilledman.cpp # src/netfulfilledman.h # src/policy/feerate.cpp # src/policy/fees.cpp # src/pow.cpp # src/primitives/transaction.h # src/privatesend/privatesend-client.cpp # src/privatesend/privatesend-client.h # src/privatesend/privatesend-server.cpp # src/privatesend/privatesend-server.h # src/privatesend/privatesend-util.cpp # src/privatesend/privatesend-util.h # src/privatesend/privatesend.cpp # src/privatesend/privatesend.h # src/protocol.cpp # src/qt/addressbookpage.cpp # src/qt/addresstablemodel.cpp # src/qt/askpassphrasedialog.cpp # src/qt/biblepay.cpp # src/qt/biblepay.qrc # src/qt/bitcoingui.cpp # src/qt/clientmodel.h # src/qt/dash_locale.qrc # src/qt/dashstrings.cpp # src/qt/forms/helpmessagedialog.ui # src/qt/forms/masternodelist.ui # src/qt/forms/optionsdialog.ui # src/qt/guiconstants.h # src/qt/guiutil.cpp # src/qt/guiutil.h # src/qt/locale/biblepay_ar.ts # src/qt/locale/biblepay_bg.ts # src/qt/locale/biblepay_de.ts # src/qt/locale/biblepay_en.ts # src/qt/locale/biblepay_es.ts # src/qt/locale/biblepay_fi.ts # src/qt/locale/biblepay_fr.ts # src/qt/locale/biblepay_it.ts # src/qt/locale/biblepay_ja.ts # src/qt/locale/biblepay_ko.ts # src/qt/locale/biblepay_nl.ts # src/qt/locale/biblepay_pl.ts # src/qt/locale/biblepay_pt.ts # src/qt/locale/biblepay_ru.ts # src/qt/locale/biblepay_sk.ts # src/qt/locale/biblepay_sv.ts # src/qt/locale/biblepay_th.ts # src/qt/locale/biblepay_tr.ts # src/qt/locale/biblepay_vi.ts # src/qt/locale/biblepay_zh_CN.ts # src/qt/locale/biblepay_zh_TW.ts # src/qt/networkstyle.cpp # src/qt/optionsdialog.cpp # src/qt/overviewpage.cpp # src/qt/paymentserver.cpp # src/qt/res/css/bezaleel.css # src/qt/res/css/dark.css # src/qt/res/css/drkblue.css # src/qt/res/css/light-hires-retro.css # src/qt/res/css/light-hires.css # src/qt/res/css/light.css # src/qt/res/icons/about.png # src/qt/res/icons/about_qt.png # src/qt/res/icons/bezaleel/drkblue_address-book.png # src/qt/res/icons/bezaleel/drkblue_editcopy.png # src/qt/res/icons/bezaleel/drkblue_editpaste.png # src/qt/res/icons/bezaleel/drkblue_remove.png # src/qt/res/icons/bezaleel/fontbigger.png # src/qt/res/icons/bezaleel/fontsmaller.png # src/qt/res/icons/bezaleel/hd_disabled.png # src/qt/res/icons/bezaleel/hd_enabled.png # src/qt/res/icons/bezaleel/network_disabled.png # src/qt/res/icons/bezaleel/transaction_abandoned.png # src/qt/res/icons/bitcoin_testnet.png # src/qt/res/icons/browse.png # src/qt/res/icons/configure.png # src/qt/res/icons/crownium/fontbigger.png # src/qt/res/icons/crownium/fontsmaller.png # src/qt/res/icons/crownium/hd_disabled.png # src/qt/res/icons/crownium/hd_enabled.png # src/qt/res/icons/crownium/network_disabled.png # src/qt/res/icons/crownium/transaction_abandoned.png # src/qt/res/icons/dacicons/drkblue_editcopy.png # src/qt/res/icons/dacicons/drkblue_remove.png # src/qt/res/icons/debugwindow.png # src/qt/res/icons/drkblue/about.png # src/qt/res/icons/drkblue/masternodes.png # src/qt/res/icons/drkblue/overview.png # src/qt/res/icons/drkblue/tx_mined.png # src/qt/res/icons/drkblue/tx_output.png # src/qt/res/icons/filesave.png # src/qt/res/icons/fontbigger.png # src/qt/res/icons/fontsmaller.png # src/qt/res/icons/hd_disabled.png # src/qt/res/icons/hd_enabled.png # src/qt/res/icons/key.png # src/qt/res/icons/light-retro/about.png # src/qt/res/icons/light-retro/masternodes.png # src/qt/res/icons/light-retro/overview.png # src/qt/res/icons/light-retro/tx_mined.png # src/qt/res/icons/light/about.png # src/qt/res/icons/light/masternodes.png # src/qt/res/icons/light/overview.png # src/qt/res/icons/light/tx_mined.png # src/qt/res/icons/network_disabled.png # src/qt/res/icons/notsynced.png # src/qt/res/icons/quit.png # src/qt/res/icons/trad/about.png # src/qt/res/icons/trad/masternodes.png # src/qt/res/icons/trad/overview.png # src/qt/res/icons/trad/receive.png # src/qt/res/icons/trad/send.png # src/qt/res/icons/trad/tx_inout.png # src/qt/res/icons/trad/tx_input.png # src/qt/res/icons/trad/tx_mined.png # src/qt/res/icons/trad/tx_output.png # src/qt/res/icons/transaction_abandoned.png # src/qt/res/images/drkblue/about.png # src/qt/res/images/drkblue/checked.png # src/qt/res/images/drkblue/drkblue_walletFrame_bg.png # src/qt/res/images/drkblue/splash.png # src/qt/res/images/drkblue/splash_testnet.png # src/qt/res/images/drkblue/unchecked.png # src/qt/res/images/light-retro/about.png # src/qt/res/images/light-retro/checked.png # src/qt/res/images/light-retro/drkblue_downArrow.png # src/qt/res/images/light-retro/drkblue_downArrow_small.png # src/qt/res/images/light-retro/drkblue_leftArrow_small.png # src/qt/res/images/light-retro/drkblue_qtreeview_selected.png # src/qt/res/images/light-retro/drkblue_rightArrow_small.png # src/qt/res/images/light-retro/drkblue_upArrow_small.png # src/qt/res/images/light-retro/drkblue_walletFrame_bg.png # src/qt/res/images/light-retro/splash.png # src/qt/res/images/light-retro/splash_testnet.png # src/qt/res/images/light-retro/unchecked.png # src/qt/res/images/light/about.png # src/qt/res/images/light/checked.png # src/qt/res/images/light/drkblue_downArrow.png # src/qt/res/images/light/drkblue_downArrow_small.png # src/qt/res/images/light/drkblue_leftArrow_small.png # src/qt/res/images/light/drkblue_qtreeview_selected.png # src/qt/res/images/light/drkblue_rightArrow_small.png # src/qt/res/images/light/drkblue_upArrow_small.png # src/qt/res/images/light/drkblue_walletFrame_bg.png # src/qt/res/images/light/splash_testnet.png # src/qt/res/images/light/unchecked.png # src/qt/res/images/trad/about.png # src/qt/res/images/trad/drkblue_downArrow.png # src/qt/res/images/trad/drkblue_downArrow_small.png # src/qt/res/images/trad/drkblue_leftArrow_small.png # src/qt/res/images/trad/drkblue_qtreeview_selected.png # src/qt/res/images/trad/drkblue_rightArrow_small.png # src/qt/res/images/trad/drkblue_upArrow_small.png # src/qt/res/images/trad/drkblue_walletFrame_bg.png # src/qt/res/images/trad/splash.png # src/qt/res/images/trad/splash_testnet.png # src/qt/res/src/spinner.png # src/qt/rpcconsole.cpp # src/qt/sendcoinsdialog.cpp # src/qt/signverifymessagedialog.cpp # src/qt/splashscreen.cpp # src/qt/test/rpcnestedtests.cpp # src/qt/test/test_main.cpp # src/qt/test/uritests.cpp # src/qt/transactiondesc.cpp # src/qt/transactionrecord.cpp # src/qt/transactiontablemodel.cpp # src/qt/walletmodel.cpp # src/rpc/blockchain.cpp # src/rpc/governance.cpp # src/rpc/masternode.cpp # src/rpc/mining.cpp # src/rpc/misc.cpp # src/rpc/net.cpp # src/rpc/protocol.cpp # src/rpc/rawtransaction.cpp # src/rpc/rpcevo.cpp # src/rpc/server.cpp # src/script/biblepayconsensus.cpp # src/script/dashconsensus.h # src/sendalert.cpp # src/spork.cpp # src/spork.h # src/test/README.md # src/test/addrman_tests.cpp # src/test/alert_tests.cpp # src/test/amount_tests.cpp # src/test/blockencodings_tests.cpp # src/test/coins_tests.cpp # src/test/crypto_tests.cpp # src/test/cuckoocache_tests.cpp # src/test/data/alertTests.raw # src/test/merkle_tests.cpp # src/test/miner_tests.cpp # src/test/netbase_tests.cpp # src/test/pmt_tests.cpp # src/test/prevector_tests.cpp # src/test/ratecheck_tests.cpp # src/test/script_tests.cpp # src/test/sighash_tests.cpp # src/test/skiplist_tests.cpp # src/test/test_coin.cpp # src/test/test_coin.h # src/test/txvalidationcache_tests.cpp # src/test/util_tests.cpp # src/test/versionbits_tests.cpp # src/torcontrol.cpp # src/txdb.cpp # src/uint256.h # src/util.cpp # src/util.h # src/validation.cpp # src/validation.h # src/version.h # src/wallet/crypter.cpp # src/wallet/db.cpp # src/wallet/rpcdump.cpp # src/wallet/rpcwallet.cpp # src/wallet/test/crypto_tests.cpp # src/wallet/wallet.cpp # src/wallet/wallet.h # src/wallet/walletdb.cpp # test/functional/fundrawtransaction.py # test/functional/import-rescan.py # test/functional/merkle_blocks.py # test/functional/multi_rpc.py # test/functional/p2p-instantsend.py # test/functional/sporks.py # test/functional/test_framework/comptool.py # test/functional/test_framework/mininode.py # test/functional/test_framework/util.py # test/functional/wallet-hd.py # test/functional/wallet.py

fa6c3de p2p: Clarify control flow in ProcessMessage() (MarcoFalke) Pull request description: `ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see bitcoin#9608). It was closed because it wasn't clear if moving each case into a function was the right approach. Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix bitcoin#13162) This patch does exactly that. Also note that this patch is a subset of previous approaches such as bitcoin#9608 and bitcoin#10145. Review suggestion: `git diff HEAD~ --function-context` Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201 Signed-off-by: cevap <dev@i2pmail.org>

maflcko added Refactoring P2P labels Aug 12, 2018

maflcko force-pushed the Mf1808-RetMsgHandler branch from fa050e4 to fa37c5b Compare August 12, 2018 12:14

DrahtBot mentioned this pull request Aug 12, 2018

Dandelion transaction relay (BIP 156) #13947

Closed

p2p: Clarify control flow in ProcessMessage()

fa6c3de

maflcko force-pushed the Mf1808-RetMsgHandler branch from fa37c5b to fa6c3de Compare August 12, 2018 19:08

DrahtBot mentioned this pull request Aug 23, 2018

Add p2p layer encryption with ECDH/ChaCha20Poly1305 #14032

Closed

1 task

jnewbery reviewed Aug 24, 2018

View reviewed changes

laanwj merged commit fa6c3de into bitcoin:master Aug 25, 2018

maflcko deleted the Mf1808-RetMsgHandler branch August 25, 2018 16:22

Bushstar mentioned this pull request Sep 4, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#359

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

p2p: Clarify control flow in ProcessMessage #13946

p2p: Clarify control flow in ProcessMessage #13946

Uh oh!

maflcko commented Aug 12, 2018 •

edited

Loading

Uh oh!

practicalswift commented Aug 12, 2018

Uh oh!

domob1812 commented Aug 12, 2018

Uh oh!

DrahtBot commented Aug 12, 2018 •

edited

Loading

Uh oh!

sipa commented Aug 12, 2018

Uh oh!

maflcko commented Aug 12, 2018

Uh oh!

Empact commented Aug 13, 2018 •

edited

Loading

Uh oh!

practicalswift commented Aug 13, 2018

Uh oh!

jnewbery left a comment

Uh oh!

jnewbery Aug 24, 2018

Uh oh!

maflcko Aug 24, 2018

Uh oh!

jnewbery Aug 24, 2018

Uh oh!

laanwj commented Aug 25, 2018

Uh oh!

JeremyRubin commented Sep 16, 2018

Uh oh!

Uh oh!

p2p: Clarify control flow in ProcessMessage #13946

p2p: Clarify control flow in ProcessMessage #13946

Uh oh!

Conversation

maflcko commented Aug 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 12, 2018

Uh oh!

domob1812 commented Aug 12, 2018

Uh oh!

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

Uh oh!

Uh oh!

sipa commented Aug 12, 2018

Uh oh!

maflcko commented Aug 12, 2018

Uh oh!

Empact commented Aug 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 13, 2018

Uh oh!

jnewbery left a comment

Choose a reason for hiding this comment

Uh oh!

jnewbery Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

maflcko Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

jnewbery Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

laanwj commented Aug 25, 2018

Uh oh!

JeremyRubin commented Sep 16, 2018

Uh oh!

Uh oh!

maflcko commented Aug 12, 2018 •

edited

Loading

DrahtBot commented Aug 12, 2018 •

edited

Loading

Empact commented Aug 13, 2018 •

edited

Loading