Prepare for non-Base58 addresses #11117

sipa · 2017-08-23T01:11:46Z

This patch removes the need for the intermediary Base58 type CBitcoinAddress, by providing {Encode,Decode,IsValid}Destination functions that directly operate on the conversion between std::strings and CTxDestination.

As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit CTxDestination<->CBitcoinAddress conversions.

This change is far from complete. In follow-ups I'd like to:

Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so.
Replace CTxDestination with a non-boost::variant version (which can be more efficient as boost::variant allocates everything on the heap, and remove the need for boost::get<...> and IsValidDestination calls everywhere).
Do the same for CBitcoinSecret, CBitcoinExtKey, and CBitcoinExtPubKey.

However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into CBitcoinAddress, but I would consider that a move in the wrong direction.

promag

There is this pattern

DecodeDestination();
if (!IsValidDestination()) {

What about:

CTxDestination dest;
if (!DecodeDestination(str, dest)) {

Also, to avoid more usages of CBitcoinAddress, add commit to move it to base58.cpp?

promag · 2017-08-23T02:47:42Z

src/bitcoin-tx.cpp

Block instead?

if (...) { throw ...; }

promag · 2017-08-23T02:59:37Z

src/rpc/rawtransaction.cpp

Change to

if (!destinations.insert(destination).second) {

promag · 2017-08-23T03:04:57Z

src/wallet/rpcwallet.cpp

Nit, kind of unrelated, use .find() to avoid 2nd lookup below.

Trying to keep this as much of a straightforward change as possible, as it may need backporting.

promag · 2017-08-23T03:07:34Z

src/wallet/rpcwallet.cpp

Follow up, should the error message be Invalid destination address?

An address is just the string-encoded form of a CTxDestination, so no. I think the "destination" name should be internal.

Off topic, just Invalid address as it seems irrelevant to have Bitcoin in the error message?

promag · 2017-08-23T03:09:28Z

src/wallet/rpcwallet.cpp

Replace with .insert().second like above.

sipa · 2017-08-23T06:38:20Z

What about:

CTxDestination dest;
if (!DecodeDestination(str, dest)) {

There are plenty of cases where the result of decoding can just be passed to something else, and Function(DecodeDestination(addr)) is far cleaner than CTxDestination dest; DecodeDestination(addr, dest); Function(dest);, so I prefer to keep that possible.

The current pattern is indeed a bit verbose, but once we rewrite CTxDestination to be a native type (rather than a boost::variant; see follow-up plans in the PR description), it could at least simplify to dest.IsValid() or so.

promag · 2017-08-23T10:10:10Z

@sipa sure, I just wanted to question that "style" because it's used elsewhere.

ACK 8363a2c.

jonasschnelli · 2017-08-23T10:45:54Z

utACK 8363a2ccaffba793068db4f97184121730707104

laanwj · 2017-08-23T20:11:20Z

Added 0.15.1 milestone

kallewoof

utACK 8363a2ccaffba793068db4f97184121730707104
LGTM. I skipped over QT stuff as I don't know it very well.

theuni

utACK other than the keyID issue.

theuni · 2017-08-24T14:22:44Z

src/qt/addresstablemodel.cpp

I've stared at this for 5 min now. What trickery made this work before?

It was creating a temporary of type CBitcoinAddress by converting the CTxDestination initializer, using the implicit CBitcoinAddress(const CTxDestination &dest) constructor. That temporary's lifetime was then extended by taking a reference to it, named address.

Interesting, I didn't know an implicit conversion could be done when extending the lifetime of a const ref. Scary!

theuni · 2017-08-24T14:42:33Z

src/qt/signverifymessagedialog.cpp

Good catch.

Wow, thanks :S

theuni · 2017-08-24T15:03:13Z

src/script/standard.cpp

Maybe add a quick comment that this is CNoDestination so it comes up when grepping for future changes.

Could use !(operator==) instead?

theuni · 2017-08-24T15:04:44Z

src/script/standard.h

got it. ignore last nit.

theuni · 2017-08-24T15:39:20Z

src/wallet/rpcwallet.cpp

It took me a while to figure out what this CBitcoinAddress was doing. Answer: nothing at all.

sipa · 2017-08-27T17:26:07Z

Removed CBitcoinAddress entirely, using a commit from #11167.

promag

Since this is the base for Bech32, maybe move everything related to CTxDestination out of base58, including tests, to a new place. Also move CTxDestination definition and related functions from src/script/standard.h?

At least should be done in a follow up?

promag · 2017-09-01T21:09:06Z

src/base58.cpp

promag · 2017-09-01T21:50:02Z

src/qt/sendcoinsdialog.cpp

Nit, fix braces.

Not going to reflow this entire function in a PR intended for backporting.

promag · 2017-09-01T21:53:32Z

src/rpc/misc.cpp

Could use currentAddress = request.params[0].get_str();?

There may be reasons not to do that. Bech32 addresses (added in a later PR) can be uppercase or lowercase, but we always want to store and look up in lowercase form. This is an easy way of doing that, and we should probably check that it's done consistently elsewhere too.

Agree, I'm +1 for clear data validation and/or sanitization. Although encode(decode(data)) is a obscure way of doing it, not to mention encode(decode(data)) should equal data?

sipa · 2017-09-03T20:23:05Z

Since this is the base for Bech32, maybe move everything related to CTxDestination out of base58, including tests, to a new place.

Yes, read the PR description. I plan to do this, but not in a patch that's intended for backporting.

Also move CTxDestination definition and related functions from src/script/standard.h?

Why? To where?

instagibbs · 2017-09-05T21:55:29Z

utACK 87d5c28

though I do not know the QT stuff well.

promag · 2017-09-05T23:41:59Z

Removed CBitcoinAddress entirely, using a commit from #11167.

@sipa missing this commit as CBitcoinAddress is still around.

sipa · 2017-09-05T23:44:45Z

@promag Yes, I removed it again.

This PR is just the part that is likely to break easily and need non-trivial rebasing.

promag · 2017-09-06T00:13:29Z

src/qt/transactiondesc.cpp

This incur in a double decoding since above IsValidDestination also decodes. Maybe swap:

CTxDestination address = DecodeDestination(rec->address); if (IsValidDestination(address)) {

This is how it's done multiples times in this PR.

laanwj · 2017-09-06T00:20:46Z

Not sure if we support this version, but I get the following compile error with clang 3.5.0:

/home/user/src/bitcoin/src/qt/signverifymessagedialog.cpp:235:40: error: invalid operands to binary expression ('CTxDestination' (aka 'variant<CNoDestination, CKeyID, CScriptID>') and 'CTxDestination')
    if (CTxDestination(pubkey.GetID()) != destination) {

Then a lot of note: candidate function not viable like:

/usr/include/x86_64-linux-gnu/qt5/QtCore/qchar.h:544:13: note: candidate function not viable: no known conversion from 'CTxDestination' (aka 'variant<CNoDestination, CKeyID, CScriptID>') to 'QChar' for 1st argument
inline bool operator!=(QChar c1, QChar c2) { return c1.unicode() != c2.unicode(); }

Edit: Ok, to answer my own question @morcos, the 0.13.0 release notes mention:

Effectively this means GCC 4.7 or higher, or Clang 3.3 or higher.

So yes we should try to support 3.5.

JeremyRubin · 2017-09-06T00:23:48Z

@laanwj

changing it to

if (!(CTxDestination(pubkey.GetID()) == destination)) {

probably fixes it. Ran into this while writing #9991

laanwj · 2017-09-06T00:30:52Z

I confirm that @JeremyRubin's workaround makes it compile.

Edit: after discussion it seems likely that this is due to a boost version difference, not a compiler difference.

See Bitcoin-ABC/bitcoin-abc@831a13b + bitcoin/bitcoin#11117

Refactor t-address encoding Includes code cherry-picked from the following upstream PRs: - bitcoin/bitcoin#11117 - bitcoin/bitcoin#11259 - Only the second commit (first is for QT code) - bitcoin/bitcoin#11167 - Only the first commit (the rest are not part of the t-address encoding refactor). Part of zcash#3058. Precursor to zcash#3202.

Based on: - bitcoin#11117 - bitcoin#11167 Related to: - https://github.com/bitcoin/bitcoin/pull/11372/filesDecodeBase58Check

864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e

* Merge bitcoin#11117: Prepare for non-Base58 addresses 864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e * CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * fix CBitcoinAddress GetKeyID check Signed-off-by: Pasta <[email protected]> * fix providertx.cpp Signed-off-by: Pasta <[email protected]> * hopefully fix governance-classes.cpp Signed-off-by: Pasta <[email protected]> * partially fix governance-validators.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <[email protected]> * partially fix governance-classes.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <[email protected]> * fix governance-classes.h Signed-off-by: Pasta <[email protected]> * DecodeTransaction -> DecodeDestination, fix governance-validators.cpp Signed-off-by: Pasta <[email protected]> * More fixes for 3294 * Move GetIndexKey into rpc/misc.cpp near getAddressesFromParams No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams Co-authored-by: Wladimir J. van der Laan <[email protected]> Co-authored-by: UdjinM6 <[email protected]> Co-authored-by: Alexander Block <[email protected]>

86e6dd4 Remove duplicate destination decoding (João Barbosa) 8d0041e Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa) Pull request description: Follow up of bitcoin#11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing bitcoin#11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`. For reference see [comment](bitcoin#11117 (comment)). Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd

86e6dd4b6 Remove duplicate destination decoding (João Barbosa) 8d0041e60 Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa) Pull request description: Follow up of #11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing #11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`. For reference see [comment](bitcoin/bitcoin#11117 (comment)). Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd

ac99512 [Model] Do not return a Destination if getNewAddress fails. (furszy) cba200c CBitcoinAddress replacement for CTxDestionation encoding in RPC files. (furszy) e319981 GUI CBitcoinAddress moved to the CTxDestination abstraction. (furszy) 6e29c1d Tier two related CBitcoinAddress moved to the CTxDestination abstraction (furszy) 8436183 getNewAddress moved to CTxDestination and Destination wrapper connected to the GUI (furszy) fd5bd3d Destination wrapper class + DestinationFor method created. (furszy) 849c03a Introduce wrappers around CBitcoinAddress (furszy) Pull request description: This patch removes the need for the intermediary Base58 type CBitcoinAddress, by providing {Encode,Decode,IsValid} Destination functions that directly operate on the conversion between std::strings and CTxDestination. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit CTxDestination<->CBitcoinAddress conversions. It's not a small change but it will give us much more flexibility once it's fully done over every current and new address type. The first commit is coming from upstream partially ([11117](bitcoin#11117)), the rest is purely tackling our code (we have so so many more places using the CBitcoinAddress class). And finally, have implemented a second abstraction (`Destination` wrapper) on top of the base58 address to {Encode,Decode,IsValid} Staking addresses as well. **Note:** This migration is not fully complete. It's first step, we still have places using the CBitcoinAddress object instead of the CTxDestination abstraction. ACKs for top commit: Fuzzbawls: ACK ac99512 random-zebra: ACK ac99512 and merging... Tree-SHA512: 94388eb487948b4600749047fd4b63d651c2da3dc34a01746a50d0ae73e4d851e1a9afe4f1a371936581368b174de178e8a0f77032ba6606657339473513f904

e07286d Remove possibly confusing IsValidDestinationString(str) (furszy) a7bafa1 Implement {Encode,Decode}Destination without CBitcoinAddress (furszy) 7d3ba5d Remove unused GetKeyID and IsScript methods from CBitcoinAddress (furszy) e7c5d9a Move CBitcoinAddress to base58.cpp (furszy) 548f828 Final migration from CBitcoinAddress to the destionation wrapper (furszy) Pull request description: Finished the complete removal of the base58 address class from the sources, migrated to the destination wrapper. Plus includes bitcoin#11117 - last commit - and bitcoin#11259 as an extra. ACKs for top commit: random-zebra: utACK e07286d Fuzzbawls: utACK e07286d Tree-SHA512: b2bf4c5ed10f863f0de7ab57a87610fddab1bbe21a1f585006e5c370ba3c04635fb1314a9d9a80b2c15a66d5bfe34fd9590dc113f0d91d93d33eb37344c7f8fe

Based on: - bitcoin#11117 - bitcoin#11167 Related to: - https://github.com/bitcoin/bitcoin/pull/11372/filesDecodeBase58Check

An overview of commits that implement an initial switch from using CBitcoinAddress to CTxDestination using bech32 (in line with Bitcoin's implementation). A 80% implementation (not cleaned up) can be found in our archives: - cevap/ion-old-v3@3b30fd0 - cevap/ion-old-v3@f8ee3a8 - cevap/ion-old-v3@7f87033 - cevap/ion-old-v3@f0d2206 - cevap/ion-old-v3@dbddc7b - cevap/ion-old-v3@e834f27 - cevap/ion-old-v3@2c467e1 - cevap/ion-old-v3@ef36201 - cevap/ion-old-v3@3d6e391 Implementing this switch in full is future work, which needs: - bitcoin#11117 - bitcoin#11167 - bitcoin#11372

Based on: - bitcoin#11117 - bitcoin#11167 Related to: - https://github.com/bitcoin/bitcoin/pull/11372/filesDecodeBase58Check

An overview of commits that implement an initial switch from using CBitcoinAddress to CTxDestination using bech32 (in line with Bitcoin's implementation). A 80% implementation (not cleaned up) can be found in our archives: - cevap/ion-old-v3@3b30fd0 - cevap/ion-old-v3@f8ee3a8 - cevap/ion-old-v3@7f87033 - cevap/ion-old-v3@f0d2206 - cevap/ion-old-v3@dbddc7b - cevap/ion-old-v3@e834f27 - cevap/ion-old-v3@2c467e1 - cevap/ion-old-v3@ef36201 - cevap/ion-old-v3@3d6e391 Implementing this switch in full is future work, which needs: - bitcoin#11117 - bitcoin#11167 - bitcoin#11372

* Merge bitcoin#11117: Prepare for non-Base58 addresses 864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e * CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <[email protected]> * fix CBitcoinAddress GetKeyID check Signed-off-by: Pasta <[email protected]> * fix providertx.cpp Signed-off-by: Pasta <[email protected]> * hopefully fix governance-classes.cpp Signed-off-by: Pasta <[email protected]> * partially fix governance-validators.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <[email protected]> * partially fix governance-classes.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <[email protected]> * fix governance-classes.h Signed-off-by: Pasta <[email protected]> * DecodeTransaction -> DecodeDestination, fix governance-validators.cpp Signed-off-by: Pasta <[email protected]> * More fixes for 3294 * Move GetIndexKey into rpc/misc.cpp near getAddressesFromParams No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams Co-authored-by: Wladimir J. van der Laan <[email protected]> Co-authored-by: UdjinM6 <[email protected]> Co-authored-by: Alexander Block <[email protected]>

promag reviewed Aug 23, 2017

View reviewed changes

sipa force-pushed the 201708_nocbitcoinaddress branch from 36eb9f2 to 8363a2c Compare August 23, 2017 06:47

laanwj added the Wallet label Aug 23, 2017

laanwj added this to the 0.15.1 milestone Aug 23, 2017

laanwj added the Needs backport label Aug 23, 2017

kallewoof approved these changes Aug 24, 2017

View reviewed changes

theuni reviewed Aug 24, 2017

View reviewed changes

sipa force-pushed the 201708_nocbitcoinaddress branch from 8363a2c to 7e7a025 Compare August 24, 2017 18:12

sipa mentioned this pull request Aug 26, 2017

Full BIP173 (Bech32) support #11167

Merged

sipa force-pushed the 201708_nocbitcoinaddress branch from 7e7a025 to 4bc71a6 Compare August 27, 2017 17:23

promag reviewed Sep 1, 2017

View reviewed changes

sipa force-pushed the 201708_nocbitcoinaddress branch 3 times, most recently from f77636b to 3144af1 Compare September 3, 2017 20:14

sipa force-pushed the 201708_nocbitcoinaddress branch from 3144af1 to 87d5c28 Compare September 5, 2017 18:53

promag reviewed Sep 6, 2017

View reviewed changes

sipa force-pushed the 201708_nocbitcoinaddress branch from 87d5c28 to 8dc36db Compare September 6, 2017 00:40

JeremyRubin mentioned this pull request Sep 6, 2017

Add != to Base58 class #11253

Closed

mkjekk mentioned this pull request May 4, 2018

Wallet crash lbtcio/lbtc-lightwallet-client#8

Open

niahmiah pushed a commit to owstack/bitcoin-abc that referenced this pull request May 14, 2018

Fix code for CBitcoinAddress wrappers

403bf99

See Bitcoin-ABC/bitcoin-abc@831a13b + bitcoin/bitcoin#11117

niahmiah pushed a commit to owstack/bitcoin-abc that referenced this pull request May 14, 2018

Fix code for CBitcoinAddress wrappers

58d700f

See Bitcoin-ABC/bitcoin-abc@831a13b + bitcoin/bitcoin#11117

niahmiah pushed a commit to owstack/bitcoin-abc that referenced this pull request May 14, 2018

Fix code for CBitcoinAddress wrappers

00ec878

See Bitcoin-ABC/bitcoin-abc@831a13b + bitcoin/bitcoin#11117

furszy mentioned this pull request May 3, 2020

Prepare for non-Base58 addresses [Step 1] PIVX-Project/PIVX#1593

Merged

furszy mentioned this pull request Jun 12, 2020

Prepare for non-Base58 addresses [Step 3] PIVX-Project/PIVX#1679

Merged

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

Prepare for non-Base58 addresses #11117

Prepare for non-Base58 addresses #11117

Uh oh!

Conversation

sipa commented Aug 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag 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!

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!

sipa commented Aug 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Aug 23, 2017

Uh oh!

jonasschnelli commented Aug 23, 2017

Uh oh!

laanwj commented Aug 23, 2017

Uh oh!

kallewoof left a comment

Choose a reason for hiding this comment

Uh oh!

theuni 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!

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!

sipa commented Aug 27, 2017

Uh oh!

promag 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!

Choose a reason for hiding this comment

Uh oh!

sipa commented Aug 23, 2017 •

edited

Loading

sipa commented Aug 23, 2017 •

edited

Loading

promag Sep 6, 2017 •

edited

Loading

laanwj commented Sep 6, 2017 •

edited

Loading

laanwj commented Sep 6, 2017 •

edited

Loading