refactor: wrap accept() and extend usage of Sock #21879

vasild · 2021-05-07T16:07:50Z

This is a piece of #21878, chopped off to ease review.

Introduce an accept(2) wrapper Sock::Accept() and extend the usage of Sock in CConnman::ListenSocket and CreateNodeFromAcceptedSocket().

DrahtBot · 2021-05-07T21:38:43Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23604 (Use Sock in CNode by vasild)
#23531 (Add Yggdrasil support by prusnak)
#21878 (Make all networking code mockable by vasild)

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.

practicalswift · 2021-05-08T13:04:39Z

Concept ACK

jonatack · 2021-05-10T10:37:44Z

Concept ACK

vasild · 2021-05-19T11:53:45Z

fb10bf2ba7...0f83150f14: rebase due to conflicts

vasild · 2021-05-20T13:35:55Z

0f83150f14...9f69aa45a8: rebase and follow the trend from #21659 and flag the newly added Sock::Accept() with [[nodiscard]]

vasild · 2021-06-03T14:39:59Z

9f69aa45a8...9f04449a8e: apply upstream changes.

dhruv

crACK tACK 9f04449

Thank you for helping decouple our tests from system calls, @vasild. I also ran tests, and manually tested incoming connections on regtest using:

# Start node0 with -listen
$ src/bitcoind -regtest -datadir=`pwd`/node0 -rpccookiefile=.cookie -port=2222 -rpcport=3333 -listen

# Generate a block on node0
$ src/bitcoin-cli -regtest -datadir=`pwd`/node0 -rpccookiefile=.cookie -rpcport=3333 createwallet test
$ src/bitcoin-cli -regtest -datadir=`pwd`/node0 -rpccookiefile=.cookie -rpcport=3333 -generate 

# node0 has a block
$ src/bitcoin-cli -regtest -datadir=`pwd`/node0 -rpccookiefile=.cookie -rpcport=3333 getblockcount
1

$ src/bitcoin-cli -regtest -datadir=`pwd`/node0 -rpccookiefile=.cookie -rpcport=3333 getbestblockhash
1878296b74ee617ff119d2fc3f85c9fb35eae952e0360e505bb7d325dfe9e825

# Start node1
$ src/bitcoind -regtest -datadir=`pwd`/node1 -rpccookiefile=.cookie -port=2223 -rpcport=3334

# node1 has no blocks
$ src/bitcoin-cli -regtest -datadir=`pwd`/node1 -rpccookiefile=.cookie -rpcport=3334 getblockcount
0

# Restart node1 with -addnode=node0
$ src/bitcoind -regtest -datadir=`pwd`/node1 -rpccookiefile=.cookie -port=2223 -rpcport=3334 -addnode=127.0.0.1:2222

# node1 completes IBD and has the block
$ src/bitcoin-cli -regtest -datadir=`pwd`/node1 -rpccookiefile=.cookie -rpcport=3334 getblockcount
1

$ src/bitcoin-cli -regtest -datadir=`pwd`/node1 -rpccookiefile=.cookie -rpcport=3334 getbestblockhash
1878296b74ee617ff119d2fc3f85c9fb35eae952e0360e505bb7d325dfe9e825

Would the goal eventually be to not have to invoke Sock::Release() anywhere and instead use move semantics across the board as more code is RAII-fied?

dhruv · 2021-06-07T22:29:54Z

src/test/util/net.h

+                *addr_len = sizeof(sockaddr_in);
+                sockaddr_in* addr_in = reinterpret_cast<sockaddr_in*>(addr);
+                addr_in->sin_family = AF_INET;
+                memset(&addr_in->sin_addr, 0x05, sizeof(addr_in->sin_addr));


(nit): Would inet_pton(AF_INET, "5.5.5.5", &addr_in->sin_addr); be clearer?

Yes, that would be clearer. However, some inet_...() function ended up using syscalls (it created sockets internally, IIRC) on Linux. @practicalswift, what do you think?

src/test/util/net.h

practicalswift · 2021-06-10T09:24:58Z

@vasild

Very nice work.

Non-blocking suggestion: Would it be possible to add a fuzzing harness utilizing FuzzedSock::Accept as part of this PR? It would be really nice to see this in action, and it would also transition FuzzedSock::Accept out of "unused code" territory :)

vasild · 2021-06-10T10:33:41Z

Would the goal eventually be to not have to invoke Sock::Release() anywhere and instead use move semantics across the board as more code is RAII-fied?

Yes! #21878 removes Sock::Release() in a later commit, called... eh... net: remove now unused Sock::Release() 😄. It uses either move semantics or passes the Sock objects by reference if a function wants to use the socket but not claim ownership of it.

Would it be possible to add a fuzzing harness utilizing FuzzedSock::Accept as part of this PR? ...

The Accept() method is only called from CConnman::AcceptConnection() which itself calls a bunch of other functions that are not mocked yet in this PR. All those are dealt with in #21878 which also adds fuzzing of CConnman::SocketHandler() (which calls CConnman::AcceptConnection()) in commit fuzz: add CConnman::SocketHandler() to the tests.

jamesob

ACK 9f04449 (jamesob/ackr/21879.1.vasild.wrap_accept_and_extend_u)

Change looks good to me and, as others have noted, making network code more mockable will make writing more comprehensive tests feasible. Cloned locally and ran unit, functional tests.

When I made the dumb mutation below, both test suites still passed so I guess we don't have any tests exercising socket errors.

diff --git a/src/util/sock.cpp b/src/util/sock.cpp
index 9a92fd1b5f..e25c1bab7f 100644
--- a/src/util/sock.cpp
+++ b/src/util/sock.cpp
@@ -76,7 +76,7 @@ std::unique_ptr<Sock> Sock::Accept(sockaddr* addr, socklen_t* addr_len) const
 #ifdef WIN32
     static constexpr auto ERR = INVALID_SOCKET;
 #else
-    static constexpr auto ERR = SOCKET_ERROR;
+    static constexpr auto ERR = SOCKET_ERROR + 1;
 #endif

     const auto socket = accept(m_socket, addr, addr_len);

Show signature data

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 9f04449a8ec7dc167a0d6d0cb47c5facd205a81a ([`jamesob/ackr/21879.1.vasild.wrap_accept_and_extend_u`](https://github.com/jamesob/bitcoin/tree/ackr/21879.1.vasild.wrap_accept_and_extend_u))

Change looks good to me and, as others have noted, making network code more mockable will make writing comprehensive tests more feasible. Cloned locally and ran unit, functional tests.

When I made the dumb mutation below, both test suites passed so I guess we don't have any tests exercising invalid socket errors.

<patch>
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmEgIVsACgkQepNdrbLE
TwWAvA/8C6OfGufmfaEdYzaaGVPyq1jmF7GmXsFo0T3AkflMBbMqri4Q+1ai1Yzs
CkVOuxGBDj8wiKxEeRsbsCsp3gbcOcp21ccvn6EQkPi0Mk8Rusj1iXgs+sDPuf7V
tKq3uQ54IbxkavKKWWhp5VeGrm4ddm8s2qF5uaN7UcGlGAPTw1915YW2HmEa3sC3
9hQb+VGw3nkczUBJXMZls0AUF6E8XYTX3QycSGjlpazgSJac5J54trfvdCzpR1Ae
0WlWndO/VROlePi6p2Mg0DHMCuuRyk78qCHx547WcaQxYWOic+XMEU1OELueLVbf
gf16So4iITHJWeK/GOLuecRfGm58uquWtQuzl5twzvPmzHRrwYyHrvgQP9eANzkQ
VCIbZWV7zL5ksgDGRJTwwzUERICvB6PqlzSU3a49f7kD2DnW+CMzHROe2YW+uLM7
lIn6fmw7WCLBAi990aOtlTf9eT4YYrkneNQYz4vnS/uWvNFBqrkCFxhTdBDHg0jr
8EDzNVFtBFnxKBLYia+ybnPb8pqqAak2AYlrpYVSw6NWSlaWLq8d5JwqkNjgY2K1
anpssdubMvHmWk+N5e0rFbRg1froCMB3Fn7BhbSRbNlnKecv1M+WE1UgI8Y4Hlcn
nLxE/xMFHD94q9N79b76ENqDN8lOgTQmIbUVVcW+LKJpZ5bIQnU=
=EpEl
-----END PGP SIGNATURE-----

Show platform data

Tested on Linux-4.19.0-17-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare --enable-wallet --with-daemon CXX=/usr/local/bin/clang++ CC=/usr/local/bin/clang --with-bignum=no PYTHONPATH= --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/local/bin/clang++ -std=c++17 -mavx -mavx2 -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare -fdebug-prefix-map=$(abs_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -msse4 -msha -msse4.1 -msse4.2  i

Compiler version: Debian clang version 11.1.0-++20210428103820+1fdec59bffc1-1~exp1~20210428204437.162

vasild · 2021-08-23T15:12:29Z

@jamesob, thanks for looking into this!

Your mod changes ERR from -1 to 0 and the code looks like this:

    const auto socket = accept(m_socket, addr, addr_len);
    if (socket == 0) {
        return nullptr;
    }
    return std::make_unique<Sock>(socket);

This will change the behavior in two cases:

If accept() succeeds and returns socket=0. Could happen if stdin (fd=0) is closed and the descriptor number 0 is reused? In this case Sock::Accept() would return nullptr, signaling failure.
If accept() fails and returns -1, then the check -1 == 0 would be false and Sock::Accept() would return a Sock object with internal m_socket equal to -1.

I think it is unlikely that any of that happens (the real syscall accept() to return -1 or 0).

jamesob · 2021-08-23T22:53:10Z

@vasild yup, it was that second case I was curious about. Nothing wrong with this PR of course, just thought it might be interesting to note that we don't seem to have any coverage for the accept()-fails branches. I think this PR will make it feasible to add such coverage, if that ends up being worth doing.

vasild · 2021-08-24T06:21:39Z

Right! CConnman::AcceptConnection() is a candidate for fuzzing once all of #21878 is merged.

vasild · 2021-08-27T14:28:03Z

This PR has 2 ACKs and 2 Concept ACKs. @practicalswift, @jonatack, willing to upgrade your "Concept ACK"s to full code review ACKs?

jonatack · 2021-08-27T14:30:14Z

Yes, I was reviewing this a few days ago when I hit the addrman corruption issue from the asmap/addrman init order. Back to it soon!

jonatack

Tested ACK 9f04449 rebased to master at a8a272a, code review, debug build and ran unit+functional tests at each commit, ran bitcoind on this branch head rebased to master on mainnet with a full range of connections including inbounds, behavior and debug log nominal

This now has 3 ACKs by @dhruv, @jamesob and me.

src/util/sock.h

vasild · 2021-09-28T07:27:01Z

9f04449a8e...78e21e511e: rebase due to a silent windows-only "conflict" and address review suggestions.

Invalidates ACKs from @dhruv, @jamesob, @jonatack.

jonatack · 2021-09-28T09:25:11Z

Diff-review re-ACK 78e21e511ee241900b84d763462f26972d83b600 per git range-diff a9d0cec 9f04449 78e21e following my previous full review (#21879 (review))

jonatack

re-ACK 0f4474d6da5741c7dc61cef403bb03faa2345f37 per git range-diff 2ef186a 07d998f 0f4474, re-review, rebase/debug build/ran units following my previous full review (#21879 (review))

vasild · 2021-11-26T11:04:27Z

0f4474d6da...976f6e8bc9: rebase due to conflicts; close the accepted socket if std::make_unique() throws.

Invalidates ACK from @jonatack

Previously invalidated ACKs from @dhruv, @jamesob

This will help to increase `Sock` usage and make more code mockable.

Change `CConnman::ListenSocket` to use a pointer to `Sock` instead of a bare `SOCKET` and use `Sock::Accept()` instead of bare `accept()`. This will help mocking / testing / fuzzing more code.

Change `CConnman::CreateNodeFromAcceptedSocket()` to take a `Sock` argument instead of `SOCKET`. This makes the method mockable and also a little bit shorter as some `CloseSocket()` calls are removed (the socket will be closed automatically by the `Sock` destructor on early return).

vasild · 2021-12-01T15:37:17Z

976f6e8bc9...6bf6e9fd9d: rebase due to conflicts

Previously invalidated ACKs from @jonatack, @dhruv, @jamesob

jamesob

ACK 6bf6e9f (jamesob/ackr/21879.2.vasild.wrap_accept_and_extend_u)

Examined interdiff and built/tested locally with a pre-populated datadir.

Changes since last review include rebasing on master (CJDNS changes, vNodes -> m_nodes), some && sock function sig changes, write_len rephrasing, and catching exceptions during unique_ptr construction in Sock::Accept.

Show signature data

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 6bf6e9fd9dece67878595a5f3361851c25833c49 ([`jamesob/ackr/21879.2.vasild.wrap_accept_and_extend_u`](https://github.com/jamesob/bitcoin/tree/ackr/21879.2.vasild.wrap_accept_and_extend_u))

Examined interdiff and built/tested locally with a pre-populated datadir.

Changes since last review include rebasing on master (CJDNS changes, vNodes -> m_nodes), some
&& sock function sig changes, `write_len` rephrasing, and catching exceptions during unique_ptr
construction in Sock::Accept.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmGn3PIACgkQepNdrbLE
TwXzpw/+KLFtcxY3gB2hbfsjsm0IUUiY/FWnFL1S5ADu5HaFAMHHkT9cEfCsMhAn
oQeGgtKJ4+kp3j01JEHwo7Sa/XLj1CEKWLbLd3OFJyFgQ7GJr0X9Ej0RmHN67Rjz
B70DzBNe6FeRz/tST+fkVOULLJqj0bwg1FeVMsHK1bbAWCtl358O25flNeXy5zCI
tY5IAevJII5RP1YjbeTddY5VRPXBNP/dohEL6M5mvZdNsz5/OHz7VfuCbJtH1LBC
E7HcaMl8y70feJ4bVHuRmupx1+TYy5JUx1Aj05EcNzmkC/APHdTcL9bsZXiiOeHU
nvp81Ozq7oRe9ZkEzTPiKScPP6rqHbFILUE+dYW22P6qjDczHdge2sPVzcyIWKMR
CW7tG9PSo4dzRly+jh8T90ApnPe7oVQr6pzPVTjAlyuNHcS7CWYnEWS0Nha1zWyv
TYV2jC/53ugrqVnUAm+HbfTR2SIhTpYgYpUkYArHmEyXL/PZOnYB02+vgEhTsMkQ
JSYHb7AwlHoKr9R+S7XvkNG2TN2/2x4XZNTYr8sDvdNlxWLzsfTbTv1g9gfX/U1U
VFvJNFUaY/i8qq0FUKYgHvz0JMvaoRnsTx5w1K4m9tvdiNtA34TJy+y1sP1VPnnr
7dA09N0rRu9BuAzioViAsd2eVZQS3TUOjvsYnKFL6EmsC/36lgg=
=HB8m
-----END PGP SIGNATURE-----

Show platform data

Tested on Linux-5.10.0-9-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare -Wsign-compare -Werror=thread-safety-analysis --enable-wallet --enable-debug --with-daemon --enable-natpmp-default CXX=/usr/bin/clang++ CC=/usr/bin/clang PYTHONPATH= --disable-openssl-tests --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental --disable-openssl-tests --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++17 -mavx -mavx2 -mpclmul -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare -Wsign-compare -Werror=thread-safety-analysis -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -msse4 -msha -msse4.1 -msse4.2  i

Compiler version: Debian clang version 13.0.1-++20211110052940+162f3f18c945-1~exp1~20211110053502.25

jonatack

ACK 6bf6e9f per git range-diff ea989de 976f6e8 6bf6e9f -- only change since my last review was s/listen_socket.socket/listen_socket.sock->Get()/ in src/net.cpp: CConnman::SocketHandlerListening() -- re-read the code changes, rebase/debug build/ran units following my previous full review (#21879 (review))

jonatack · 2021-12-07T21:12:12Z

Could maybe use another pair of eyes on this, but otherwise it seems RFM.

maflcko · 2021-12-08T08:34:04Z

It is unclear from the subject line and labels, but this is refactoring? It might be good to clarify that in the commit messages (if you need to push again) or at least in the pull request title and labels. This makes it easier for reviewers to see if it was accidental when it later turns out that this introduced any bugs or features.

vasild · 2021-12-08T09:03:46Z

Yes, this is refactor since it does not change external behavior. Updated the PR title. I can't edit labels.

w0xlt

tACK 6bf6e9f

Ran the node on signet and the unit / functional tests.

Seems good. Sock::Accept encapsulates the accept() system call, making the code more mockable and simpler due to the use of the RAII Sock class.

If I understand correctly, instead of manually closing each socket from vhListenSocket, Sock destructor will automatically handle this after vhListenSocket.clear().

vasild · 2022-01-04T15:54:02Z

If I understand correctly, instead of manually closing each socket from vhListenSocket, Sock destructor will automatically handle this after vhListenSocket.clear().

Yes!

laanwj · 2022-01-05T14:29:35Z

Code review ACK 6bf6e9f

vasild mentioned this pull request May 7, 2021

Make all networking code mockable #21878

Closed

12 tasks

DrahtBot added P2P Utils/log/libs labels May 7, 2021

DrahtBot mentioned this pull request May 7, 2021

p2p, refactor: make NetPermissionFlags an enum class #21506

Merged

DrahtBot added the Needs rebase label May 19, 2021

vasild force-pushed the SockAccept branch from fb10bf2 to 0f83150 Compare May 19, 2021 11:53

DrahtBot removed the Needs rebase label May 19, 2021

vasild force-pushed the SockAccept branch from 0f83150 to 9f69aa4 Compare May 20, 2021 13:34

vasild force-pushed the SockAccept branch from 9f69aa4 to 9f04449 Compare June 3, 2021 14:38

dhruv reviewed Jun 9, 2021

View reviewed changes

jamesob approved these changes Aug 20, 2021

View reviewed changes

DrahtBot mentioned this pull request Aug 30, 2021

refactor: various RecursiveMutex replacements in CConnman #22829

Merged

jonatack approved these changes Sep 22, 2021

View reviewed changes

src/util/sock.h Show resolved Hide resolved

DrahtBot mentioned this pull request Sep 23, 2021

Full CJDNS support #23077

Merged

vasild force-pushed the SockAccept branch from 9f04449 to 78e21e5 Compare September 28, 2021 07:25

jonatack reviewed Nov 17, 2021

View reviewed changes

DrahtBot added the Needs rebase label Nov 24, 2021

vasild force-pushed the SockAccept branch from 0f4474d to 976f6e8 Compare November 26, 2021 10:59

DrahtBot removed the Needs rebase label Nov 26, 2021

This was referenced Nov 27, 2021

Use Sock in CNode #23604

Merged

Don't check if the listening socket is valid #23601

Merged

DrahtBot added the Needs rebase label Dec 1, 2021

vasild added 3 commits December 1, 2021 15:22

net: add new method Sock::Accept() that wraps accept()

f8bd13f

This will help to increase `Sock` usage and make more code mockable.

net: use Sock in CConnman::ListenSocket

9e3cbfc

Change `CConnman::ListenSocket` to use a pointer to `Sock` instead of a bare `SOCKET` and use `Sock::Accept()` instead of bare `accept()`. This will help mocking / testing / fuzzing more code.

vasild force-pushed the SockAccept branch from 976f6e8 to 6bf6e9f Compare December 1, 2021 15:36

DrahtBot removed the Needs rebase label Dec 1, 2021

jamesob approved these changes Dec 1, 2021

View reviewed changes

jonatack approved these changes Dec 5, 2021

View reviewed changes

vasild changed the title ~~Wrap accept() and extend usage of Sock~~ refactor: wrap accept() and extend usage of Sock Dec 8, 2021

maflcko added Refactoring and removed Utils/log/libs labels Dec 8, 2021

DrahtBot mentioned this pull request Dec 18, 2021

docs: avoid C-style casts; use modern C++ casts #23810

Merged

w0xlt approved these changes Dec 26, 2021

View reviewed changes

laanwj merged commit 8f1c28a into bitcoin:master Jan 5, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 6, 2022

Merge bitcoin#21879: refactor: wrap accept() and extend usage of Sock

ce08540

vasild deleted the SockAccept branch January 6, 2022 09:16

bitcoin locked and limited conversation to collaborators Jan 6, 2023

refactor: wrap accept() and extend usage of Sock #21879

refactor: wrap accept() and extend usage of Sock #21879

Uh oh!

Conversation

vasild commented May 7, 2021

Uh oh!

DrahtBot commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

practicalswift commented May 8, 2021

Uh oh!

jonatack commented May 10, 2021

Uh oh!

vasild commented May 19, 2021

Uh oh!

vasild commented May 20, 2021

Uh oh!

vasild commented Jun 3, 2021

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruv Jun 7, 2021

Choose a reason for hiding this comment

Uh oh!

vasild Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

practicalswift commented Jun 10, 2021

Uh oh!

vasild commented Jun 10, 2021

Uh oh!

jamesob left a comment

Choose a reason for hiding this comment

Uh oh!

vasild commented Aug 23, 2021

Uh oh!

jamesob commented Aug 23, 2021

Uh oh!

vasild commented Aug 24, 2021

Uh oh!

vasild commented Aug 27, 2021

Uh oh!

jonatack commented Aug 27, 2021

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasild commented Sep 28, 2021

Uh oh!

jonatack commented Sep 28, 2021

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

vasild commented Nov 26, 2021

Uh oh!

vasild commented Dec 1, 2021

Uh oh!

jamesob left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack commented Dec 7, 2021

Uh oh!

maflcko commented Dec 8, 2021

Uh oh!

vasild commented Dec 8, 2021

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

vasild commented Jan 4, 2022

Uh oh!

DrahtBot commented May 7, 2021 •

edited

Loading

dhruv left a comment •

edited

Loading