refactor: Use uint16_t instead of unsigned short #19314

renepickhardt · 2020-06-17T20:21:04Z

I wanted to see if the up for grabs label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing.

So I checked out the remote branch rebased it resolved three conflicts and continued the rebase.

Hope everything is as expected (:

practicalswift · 2020-06-17T20:29:26Z

Concept ACK: explicit is better than implicit.

Warm welcome as a contributor @renepickhardt - I'm a big fan of your Lightning work etc :) Looking forward to reading Mastering the Lightning Network once it is released!

FWIW, we are already assuming that sizeof(uint16_t) == sizeof(unsigned short) implicitly in multiple places like here …

bitcoin/src/serialize.h

Lines 265 to 278 in 35ed88f

    
           /** 
        
            * Compact Size 
        
            * size <  253        -- 1 byte 
        
            * size <= USHRT_MAX  -- 3 bytes  (253 + 2 bytes) 
        
            * size <= UINT_MAX   -- 5 bytes  (254 + 4 bytes) 
        
            * size >  UINT_MAX   -- 9 bytes  (255 + 8 bytes) 
        
            */ 
        
           inline unsigned int GetSizeOfCompactSize(uint64_t nSize) 
        
           { 
        
               if (nSize < 253)             return sizeof(unsigned char); 
        
               else if (nSize <= std::numeric_limits<unsigned short>::max()) return sizeof(unsigned char) + sizeof(unsigned short); 
        
               else if (nSize <= std::numeric_limits<unsigned int>::max())  return sizeof(unsigned char) + sizeof(unsigned int); 
        
               else                         return sizeof(unsigned char) + sizeof(uint64_t); 
        
           }

… and also explicitly in assumptions.h …

bitcoin/src/compat/assumptions.h

Line 51 in 35ed88f

static_assert(sizeof(short) == 2, "16-bit short assumed");

renepickhardt · 2020-06-17T20:29:45Z

btw running git grep "unsigned short" results in:

src/secp256k1/src/tests.c:            unsigned short sh;

I could make another commit to this branch where I also changed that but I am not sure if the bitcoin repo is the place to patch libsecp256k1 or if that should be done in https://github.com/bitcoin-core/secp256k1

at least the remote file https://github.com/bitcoin-core/secp256k1/blob/master/src/tests.c has 19 contributers according to git blame where in this repo it only has contributes from @sipa and @laanwj

DrahtBot · 2020-06-17T23:11:45Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

net: Make DNS lookup mockable, add fuzzing harness #19415 (net: Make DNS lookup mockable, add fuzzing harness by practicalswift)
net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. #19203 (net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket. Add thin SOCKET wrapper. by practicalswift)
Implement ADDRv2 support (part of BIP155) #19031 (Implement ADDRv2 support (part of BIP155) by vasild)
Cache responses to GETADDR to prevent topology leaks #18991 (Cache responses to GETADDR to prevent topology leaks by naumenkogs)
Add BIP324 encrypted p2p transport de-/serializer (only used in tests) #18242 (Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli)
net: Add NAT-PMP port forwarding support #18077 (net: Add NAT-PMP port forwarding support by hebasto)
qt, refactor: Make BitcoinUnits::Unit a scoped enum #17877 (qt, refactor: Make enums in BitcoinUnits class scoped by hebasto)
p2p: Try to preserve outbound block-relay-only connections during restart #17428 (p2p: Try to preserve outbound block-relay-only connections during restart by hebasto)

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

adamjonas · 2020-06-18T15:39:26Z

src/qt/optionsmodel.h

@@ -6,6 +6,7 @@
 #define BITCOIN_QT_OPTIONSMODEL_H

 #include <amount.h>
+#include <cstdint> 


It looks like the linter is calling out the trailing whitespace here.

practicalswift · 2020-06-21T21:10:54Z

@renepickhardt Would you mind squashing? I think this one should be ready for final review after squash :)

removed trailing whitespace to make linter happy

renepickhardt · 2020-06-22T10:15:39Z

Ok so I have squashed the two commits and rebased to current master to make merging easier (:

if there is anything more to do let me know @practicalswift

sipsorcery · 2020-06-23T20:15:53Z

ACK 1cabbdd.

Nice to replace a C header with a C++ header as well.

practicalswift · 2020-06-23T20:49:27Z

ACK 1cabbdd -- patch looks correct :)

If anyone wants to follow up by checking potentially unintended/unwarranted uses of long this command might be helpful to find candidates :)

$ git grep -E '[^a-zA-Z_."]long[^a-zA-Z_."]' -- "*.cpp" "*.h" ":(exclude)src/univalue/" ":(exclude)src/qt/" ":(exclude)src/test/" ":(exclude)src/crc32c/" ":(exclude)src/crypto/common.h" ":(exclude)src/randomenv.cpp" | grep -vE '^[^ ]*:.*[/*]' | grep -vE "(too.long|ftell|strtoul|strol|strtol|how long|really long|fseek|as long)"
src/httpserver.cpp:    int workQueueDepth = std::max((long)gArgs.GetArg("-rpcworkqueue", DEFAULT_HTTP_WORKQUEUE), 1L);
src/httpserver.cpp:    int rpcThreads = std::max((long)gArgs.GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L);
src/rest.cpp:            if (headers.size() == (unsigned long)count)
src/streams.h:        long nLongPos = nPos;
src/txmempool.cpp:                if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) {
src/txmempool.h:    unsigned long size() const

luke-jr · 2020-06-24T01:34:20Z

Note the (mentioned in comments only, not an issue for this commit) libsecp256k1 unsigned short does in fact need to be an unsigned short...

luke-jr · 2020-06-24T01:35:51Z

src/addrdb.cpp

@@ -36,7 +37,7 @@ template <typename Data>
 bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data)
 {
    // Generate random temporary filename
-    unsigned short randv = 0;
+    uint16_t randv = 0;
    GetRandBytes((unsigned char*)&randv, sizeof(randv));
    std::string tmpfn = strprintf("%s.%04x", prefix, randv);


This is okay only because strprintf has some type magic.

Yes, this isn't a problem, strprintf is type safe. There is never a need for size specifiers.

luke-jr · 2020-06-24T01:36:25Z

src/net.cpp

 {
-    return (unsigned short)(gArgs.GetArg("-port", Params().GetDefaultPort()));
+    return (uint16_t)(gArgs.GetArg("-port", Params().GetDefaultPort()));


Can we just drop the cast entirely? Not sure I see the point...

The point is to make a potentially sign-changing narrowing conversion explicit.

GetArg returns int64_t so it makes sense to make the implicit conversion to uint16_t explicit using a cast.

luke-jr · 2020-06-24T01:38:49Z

src/netbase.cpp

@@ -798,11 +799,11 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int
        ProxyCredentials random_auth;
        static std::atomic_int counter(0);
        random_auth.username = random_auth.password = strprintf("%i", counter++);
-        if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) {
+        if (!Socks5(strDest, (uint16_t)port, &random_auth, hSocket)) {


Why not change the type of port?

hebasto

Approach ACK 1cabbdd.

In addition to @luke-jr's comment, could the type of the port parameter in the Socks5() function be changed from int to uint16_t?

laanwj · 2020-07-09T14:36:00Z

ACK 1cabbdd
After this, the only mention of unsigned short left is in the secp256k1 tests, which should not be changed here

In addition to @luke-jr's comment, could the type of the port parameter in the Socks5() function be changed from int to uint16_t?

I understand why @renepickhardt didn't do this. Right now, this is a pure refactor, only changing a type to a typedef of the same type (for, in practice, all architectures supported by bitcoin). Changing the parameter type would be an actual code change.

hebasto

In addition to @luke-jr's comment, could the type of the port parameter in the Socks5() function be changed from int to uint16_t?

I understand why @renepickhardt didn't do this. Right now, this is a pure refactor, only changing a type to a typedef of the same type (for, in practice, all architectures supported by bitcoin). Changing the parameter type would be an actual code change.

Agree.

ACK 1cabbdd, I have reviewed the code and it looks OK, I agree it can be merged.

1cabbdd refactor: Use uint16_t instead of unsigned short (Aaron Hook) Pull request description: I wanted to see if the `up for grabs` label works and looked at PR bitcoin#17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing. So I checked out the remote branch rebased it resolved three conflicts and continued the rebase. Hope everything is as expected (: ACKs for top commit: sipsorcery: ACK 1cabbdd. practicalswift: ACK 1cabbdd -- patch looks correct :) laanwj: ACK 1cabbdd hebasto: ACK 1cabbdd, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6

16791f2 CMakeLists tests: add raw files generation. (furszy) 672d9a2 init: move asmap code earlier in init process (Jon Atack) 65cd143 net: extract conditional to bool CNetAddr::IsHeNet (Jon Atack) 2fc1f37 logging: asmap logging and #include fixups (Jon Atack) 0c9efb8 test: add functional test for an empty, unparsable asmap (Jon Atack) 6545656 config: separate the asmap finding and parsing checks (Jon Atack) 618b8d1 config: enable passing -asmap an absolute file path (Jon Atack) 8c7bdbe config: use default value in -asmap config (Jon Atack) de39fab test: add feature_asmap functional tests (Jon Atack) 4290d3f Make asmap Interpret tolerant of malicious map data (Pieter Wuille) e527e04 Use ASNs for mapped IPv4 addresses correctly (Pieter Wuille) 9a28bc0 Mark asmap const in statistics code (Pieter Wuille) 868a6ed Avoid asmap copies in initialization (Pieter Wuille) cb698fb Add extra logging of asmap use and bucketing (Gleb Naumenko) 2fe5a05 Return mapped AS in RPC call getpeerinfo (Gleb Naumenko) ce7aa15 scripted-diff: Replace NET_TOR with NET_ONION (wodry) 4c3ae7d Integrate ASN bucketing in Addrman and add tests (Gleb Naumenko) 718f1df CAddrManTest: remove redundant MakeDeterministic call. (furszy) fd51941 Tests: address placement should be deterministic by default (René Nyffenegger) 8d01cbd Add asmap utility which queries a mapping (Gleb Naumenko) e986ed0 CAddrMan::Deserialize handle corrupt serializations better. (Patrick Strateman) d2a8baf addrman.h: CAddrInfo inline members default values, plus several typos corrected. (furszy) a7b9fd9 refactor: Use uint16_t instead of unsigned short (furszy) Pull request description: Decoupled from #2411, built on top of #2479. Probably the last decouple from the "road to Tor" work. Focused on porting the ASN nodes bucketing functionality. The hearth of this work is bitcoin#16702. Providing an asmap file that contains the IP->ASN mapping, nodes will be bucketed by AS they belong to, in order to make impossible for a node to connect to several nodes hosted in a single AS. This is done in response to Erebus attack, but also to generally diversify the connections every node creates, especially useful when a large fraction of nodes operate under a couple of cloud providers. #### List of PRs: * bitcoin#7932 * bitcoin#10765 * bitcoin#13532 * bitcoin#13575 * bitcoin#16702 * bitcoin#17812 * bitcoin#18023 * bitcoin#19314 PRs for a follow up PR: * bitcoin#18029 * bitcoin#18512 ACKs for top commit: random-zebra: re-utACK 16791f2 Fuzzbawls: ACK 16791f2 Tree-SHA512: 1452af87d693526d3359822845bbd6211578b5c7c69d740d19c8c3ee25c66fd6e130f4421066a8f5384d62f65a2754423c633f90d7e3d809f4f1cc00c3c956ba

ecde04a [Consensus] Bump Active Protocol version to 70923 for v5.3 (random-zebra) b63e4f5 Consensus: Add v5.3 enforcement height for testnet. (furszy) f44be94 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille) 015298c fix: tor: Call event_base_loopbreak from the event's callback (furszy) 34ff7a8 Consensus: Add mnb ADDRv2 guard. (furszy) b4515dc GUI: Present v3 onion addresses properly in MNs list. (furszy) 337d43d tests: don't export in6addr_loopback (Vasil Dimov) 2cde8e0 GUI: Do not show the tor v3 onion address in the topbar. (furszy) 0b5f406 Doc: update tor.md with latest upstream information. (furszy) 89df7f2 addrman: ensure old versions don't parse peers.dat (Vasil Dimov) bb90c5c test: add getnetworkinfo network name regression tests (Jon Atack) d8e01b5 rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack) 57fc7b0 net: update GetNetworkName() with all enum Network cases (Jon Atack) 647d60b tests: Modify rpc_bind to conform to bitcoin#14532 behaviour. (Carl Dong) d4d6729 Allow running rpc_bind.py --nonloopback test without IPv6 (Kristaps Kaupe) 4a034d8 test: Add rpc_bind test to default-run tests (Wladimir J. van der Laan) 61a08af [tests] bind functional test nodes to 127.0.0.1 Prevents OSX firewall (Sjors Provoost) 6a4f1e0 test: Add basic addr relay test (furszy) 78aa61c net: Make addr relay mockable (furszy) ba954ca Send and require SENDADDRV2 before VERACK (Pieter Wuille) 61c2ed4 Bump net protocol version + don't send 'sendaddrv2' to pre-70923 software (furszy) ccd508a tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov) 6da9a14 net: advertise support for ADDRv2 via new message (furszy) e58d5d0 Migrate to test_large_inv() to Misbehaving logging. (furszy) d496b64 [QA] fix mininode CAddress ser/deser (Jonas Schnelli) cec9567 net: CAddress & CAddrMan: (un)serialize as ADDRv2 Change the serialization of `CAddrMan` to serialize its addresses in ADDRv2/BIP155 format by default. Introduce a new `CAddrMan` format version (3). (furszy) b8c1dda streams update: get rid of nType and nVersion. (furszy) 3eaa273 Support bypassing range check in ReadCompactSize (Pieter Wuille) a237ba4 net: recognize TORv3/I2P/CJDNS networks (Vasil Dimov) 8e50853 util: make EncodeBase32 consume Spans (Sebastian Falbesoner) 1f67e30 net: CNetAddr: add support to (un)serialize as ADDRv2 (Vasil Dimov) 2455420 test: move HasReason so it can be reused (furszy) d41adb4 util: move HasPrefix() so it can be reused (Vasil Dimov) f6f86af Unroll Keccak-f implementation (Pieter Wuille) 45222e6 Implement keccak-f[1600] and SHA3-256 (Pieter Wuille) 08ad06d net: change CNetAddr::ip to have flexible size (furszy) 3337219 net: improve encapsulation of CNetAddr. (furszy) 910d5c4 test: Do not instantiate CAddrDB for static call (Hennadii Stepanov) 6b607ef Drop IsLimited in favor of IsReachable (Ben Woosley) a40711b IsReachable is the inverse of IsLimited (DRY). Includes unit tests (marcaiaf) 8839828 net: don't accept non-left-contiguous netmasks (Vasil Dimov) 5d7f864 rpcbind: Warn about exposing RPC to untrusted networks (Luke Dashjr) 2a6abd8 CNetAddr: Add IsBindAny method to check for INADDR_ANY (Luke Dashjr) 4fdfa45 net: Always default rpcbind to localhost, never "all interfaces" (Luke Dashjr) 31064a8 net: Minor accumulated cleanups (furszy) 9f9c871 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) f6c52a3 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) a751b9b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (furszy) f30869d test: add IsRFC2544 tests (Mark Tyneway) ed5abe1 Net: Proper CService deserialization + GetIn6Addr return false if addr isn't an IPv6 addr (furszy) 86d73fb net: save the network type explicitly in CNetAddr (Vasil Dimov) ad57dfc net: document `enum Network` (Vasil Dimov) cb160de netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) c3c04e4 net: Better misbehaving logging (furszy) 3660487 net: Use C++11 member initialization in protocol (Marco) 082baa3 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov) e2d776a util: CBufferedFile fixes (Larry Ruane) 6921f42 streams: backport OverrideStream class (furszy) Pull request description: Conjunction of a large number of back ports, updates and refactorings that made with the final goal of implementing v3 Onion addresses support (BIP155 https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) before the tor v2 addresses EOL, scheduled, by the Tor project, for (1) July 15th: v2 addr support removal from the code base, and (2) October 15th: v2 addr network disable, where **every peer in our network running under Tor will loose the connection and drop the network**. As BIP155 describes, this is introducing a new P2P message to gossip longer node addresses over the P2P network. This is required to support new-generation Onion addresses, I2P, and potentially other networks that have longer endpoint addresses than fit in the 128 bits of the current addr message. In order to achieve the end goal, had to: 1. Create Span class and push it up to latest Bitcoin implementation. 2. Update the whole serialization framework and every object using it up to latest Bitcoin implementation (3-4 years ahead of what we currently are in master). 3. Update the address manager implementing ASN-based bucketing of the network nodes. 4. Update and refactor the netAddress and address manager tests to latest Bitcoin implementation (4 years ahead of what we currently are in master). 5. Several util string, vector, encodings, parsing, hashing backports and more.. Important note: This PR it is not meant to be merged as a standalone PR, will decouple smaller ones moving on. Adding on each sub-PR its own description isolated from this big monster. Second note: This is still a **work-in-progress**, not ready for testing yet. I'm probably missing to mention few PRs that have already adapted to our sources. Just making it public so can decouple the changes, we can start merging them and i can continue working a bit more confortable (rebase a +170 commits separate branch is not fun..). ### List of back ported and adapted PRs: Span and Serialization: ---------------- * bitcoin#12886. * bitcoin#12916. * bitcoin#13558. * bitcoin#13697. (Only Span's commit 29943a9) * bitcoin#17850. * bitcoin#17896. * bitcoin#12752. * bitcoin#16577. * bitcoin#16670. (without faebf62) * bitcoin#17957. * bitcoin#18021. * bitcoin#18087. * bitcoin#18112 (only from 353f376 that we don't support). * bitcoin#18167. * bitcoin#18317. * bitcoin#18591 (only Span's commit 0fbde48) * bitcoin#18468. * bitcoin#19020. * bitcoin#19032. * bitcoin#19367. * bitcoin#19387. Net, NetAddress and AddrMan: ---------------- * bitcoin#7932. * bitcoin#10756. * bitcoin#10765. * bitcoin#12218. * bitcoin#12855. * bitcoin#13532. * bitcoin#13575. * bitcoin#13815. * bitcoin#14532. * bitcoin#15051. * bitcoin#15138. * bitcoin#15689. * bitcoin#16702. * bitcoin#17243. * bitcoin#17345. * bitcoin#17754. * bitcoin#17758. * bitcoin#17812. * bitcoin#18023. * bitcoin#18454. * bitcoin#18512. * bitcoin#19314. * bitcoin#19687 Keys and Addresses encoding: ---------------- * bitcoin#11372. * bitcoin#17511. * bitcoin#17721. Util: ---------------- * bitcoin#9140. * bitcoin#16577. * bitcoin#16889. * bitcoin#19593. Bench: ---------------- * bitcoin#16299. BIP155: ---------------- * bitcoin#19351. * bitcoin#19360. * bitcoin#19534. * bitcoin#19628. * bitcoin#19841. * bitcoin#19845. * bitcoin#19954. * bitcoin#19991 (pending). * bitcoin#19845. * bitcoin#20000 (pending). * bitcoin#20120. * bitcoin#20284. * bitcoin#20564. * bitcoin#21157 (pending). * bitcoin#21564 (pending). * Fully removed v2 onion addr support. * Add hardcoded seeds. * Add release-notes, changes to files.md and every needed documentation. I'm currently working on the PRs marked as "pending", this isn't over, but I'm pretty pretty close :). What a long road.. ACKs for top commit: random-zebra: utACK ecde04a Fuzzbawls: ACK ecde04a Tree-SHA512: 82c95fbda76fce63f96d8a9af7fa9a89cb1e1b302b7891e27118a6103af0be23606bf202c7332fa61908205e6b6351764e2ec23d753f1e2484028f57c2e8b51a

Summary: This is a backport of [[bitcoin/bitcoin#19314 | core#19314]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: deadalnix, Fabien Differential Revision: https://reviews.bitcoinabc.org/D9987

DrahtBot added GUI P2P Refactoring Tests labels Jun 17, 2020

adamjonas reviewed Jun 18, 2020

View reviewed changes

refactor: Use uint16_t instead of unsigned short

1cabbdd

removed trailing whitespace to make linter happy

renepickhardt force-pushed the akh_uint16_t branch from 4333093 to 1cabbdd Compare June 22, 2020 10:13

luke-jr reviewed Jun 24, 2020

View reviewed changes

hebasto reviewed Jun 29, 2020

View reviewed changes

DrahtBot mentioned this pull request Jun 30, 2020

net: Make DNS lookup mockable, add fuzzing harness #19415

Merged

fanquake removed the GUI label Jul 9, 2020

hebasto approved these changes Jul 9, 2020

View reviewed changes

laanwj merged commit 0d69fdb into bitcoin:master Jul 9, 2020

furszy mentioned this pull request Jun 7, 2021

Road to Tor v3 support (BIP155) PIVX-Project/PIVX#2411

Merged

furszy mentioned this pull request Jul 15, 2021

[Net] asmap to improve IP bucketing in addrman - backports PIVX-Project/PIVX#2480

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Use uint16_t instead of unsigned short #19314

refactor: Use uint16_t instead of unsigned short #19314

renepickhardt commented Jun 17, 2020 •

edited by fanquake

Loading

practicalswift commented Jun 17, 2020 •

edited

Loading

renepickhardt commented Jun 17, 2020

DrahtBot commented Jun 17, 2020 •

edited

Loading

adamjonas Jun 18, 2020

renepickhardt Jun 18, 2020

practicalswift commented Jun 21, 2020

renepickhardt commented Jun 22, 2020

sipsorcery commented Jun 23, 2020

practicalswift commented Jun 23, 2020 •

edited

Loading

luke-jr commented Jun 24, 2020 •

edited

Loading

luke-jr Jun 24, 2020

laanwj Jul 9, 2020

luke-jr Jun 24, 2020

practicalswift Jun 24, 2020

luke-jr Jun 24, 2020

hebasto left a comment

laanwj commented Jul 9, 2020

hebasto left a comment

refactor: Use uint16_t instead of unsigned short #19314

refactor: Use uint16_t instead of unsigned short #19314

Conversation

renepickhardt commented Jun 17, 2020 • edited by fanquake Loading

practicalswift commented Jun 17, 2020 • edited Loading

renepickhardt commented Jun 17, 2020

DrahtBot commented Jun 17, 2020 • edited Loading

Conflicts

adamjonas Jun 18, 2020

Choose a reason for hiding this comment

renepickhardt Jun 18, 2020

Choose a reason for hiding this comment

practicalswift commented Jun 21, 2020

renepickhardt commented Jun 22, 2020

sipsorcery commented Jun 23, 2020

practicalswift commented Jun 23, 2020 • edited Loading

luke-jr commented Jun 24, 2020 • edited Loading

luke-jr Jun 24, 2020

Choose a reason for hiding this comment

laanwj Jul 9, 2020

Choose a reason for hiding this comment

luke-jr Jun 24, 2020

Choose a reason for hiding this comment

practicalswift Jun 24, 2020

Choose a reason for hiding this comment

luke-jr Jun 24, 2020

Choose a reason for hiding this comment

hebasto left a comment

Choose a reason for hiding this comment

laanwj commented Jul 9, 2020

hebasto left a comment

Choose a reason for hiding this comment

renepickhardt commented Jun 17, 2020 •

edited by fanquake

Loading

practicalswift commented Jun 17, 2020 •

edited

Loading

DrahtBot commented Jun 17, 2020 •

edited

Loading

practicalswift commented Jun 23, 2020 •

edited

Loading

luke-jr commented Jun 24, 2020 •

edited

Loading