Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131667)
updated but worded it a bit differently - the 50% is only true if there are matches in both tables (with network interactions). lmk if the new language seems reasonable to you
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131673)
done
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131714)
done
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131741)
done
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131754)
done
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131791)
ah, thanks. updated lots of comments, I think I caught them all?
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131815)
thanks, updated this in lots of places
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131847)
incorporated this in many places in the tests I touched. definitely reads better
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131864)
added
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131874)
done
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131929)
great idea ! updated to use this while loop. also added you as co-author for this commit btw, thanks for the in depth review :)
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131961)
that makes sense, updated to remove correctness tests
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1133131985)
yeah it was, but updated to remove that assertion.
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1464978015)
thank you for the in-depth review @vasild ! I have addressed all your review comments (incorporated most, left questions on a couple)
💬 1440000bytes commented on issue "Permission to comment on PR 27235":
(https://github.com/bitcoin/bitcoin/issues/27243#issuecomment-1464984861)
I will get back to this drama in a few days. I have flight in a few hours.

Only 2 things that I wanted to comment in the CENSORED pull request of BITCOIN CORE:

1. Steps to reproduce the issue?

2. https://github.com/bitcoin/bitcoin/pull/27050 is not likely to get merged which was mentioned in [description](https://i.imgur.com/iPkMtm6.png)

3. If this is an issue, 6 devs reviewed https://github.com/bitcoin/bitcoin/pull/25315. Do we have any plans to avoid this?

If nothing works. I t
...
💬 dergoegge commented on issue "Permission to comment on PR 27235":
(https://github.com/bitcoin/bitcoin/issues/27243#issuecomment-1464990935)
There really is no drama for you to get back to here. I blocked you and I will not be unblocking you. You have a habit of posting rude and spammy comments which I find incredibly annoying. In my opinion, some of the things you have said in the past would warrant a permanent ban.
💬 clintonimaroo commented on issue "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/27242#issuecomment-1464992721)
This should help you resolve this issue, I had the same problem

1. Add the -fPIC option to the CXXFLAGS and LDFLAGS variables in the Makefile generated by the configure script.

2. Run "make clean" followed by "make" to rebuild the LevelDB library.

3. Rebuild your application with the newly constructed LevelDB library.
💬 clintonimaroo commented on issue "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/27242#issuecomment-1464993103)
> ### Current behaviour
> Building with the `--with-experimental-kernel-lib` configuration option currently leads to linker failures on OpenBSD 7.2. The following compiler/linker/build-tool versions are used:
>
> * clang 13.0.0
> * lld 13.0.0
> * ccache 4.6.3
> * GNU make 4.3
> * autoconf 2.71
> * automake 1.16.5
>
> ### Expected behaviour
> * Building should succeed
>
> ### Steps to reproduce
> ```
> $ ./configure --enable-suppress-external-warnings --with-experimental-kernel-
...
💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1465033134)
> It would be good if the functional test actually verified that the values are correctly calculated after creating a few txns; ATM it is only really a smoke test that verifies the output structure. (by @jonatack)
> Test currently only cover the count and not the fees or sizes per feerate group. (by @0xB10C)

Added explicit assert lines in the test for this.
💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1465035161)
> Concept ACK!
>
> It might make sense to have the API be more similar to https://numpy.org/doc/stable/reference/generated/numpy.histogram.html

Honestly, I'm not sure. Anyone else who prefer that approach?