Bitcoin Core Github
42 subscribers
124K links
Download Telegram
💬 hebasto commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2112971987)
> The windows failure is:
>
> ```
> D:\a\bitcoin\bitcoin\src\rpc\node.cpp(223,45): error C2065: 'CLIENT_VERSION_RC': undeclared identifier [D:\a\bitcoin\bitcoin\build_msvc\libbitcoin_node\libbitcoin_node.vcxproj]
> ```

Should fixed with:
```diff
--- a/build_msvc/bitcoin_config.h.in
+++ b/build_msvc/bitcoin_config.h.in
@@ -11,6 +11,9 @@
/* Version is release */
#define CLIENT_VERSION_IS_RELEASE $

+/* Release candidate */
+#define CLIENT_VERSION_RC $
+
/* Major version */

...
💬 apoelstra commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2112984373)
Definite concept ACK from me.
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601946313)
As for `cs_main`, it seems necessary for the chain tip to be what we think it is when the callback happens. IIUC the logging only happens in debug?
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2113010122)
> > Do we have charts anywhere tracking % of transactions that are in a cluster size of 1?
>
> I will analyze the percentage of cluster size 1 transactions mined in previous blocks.

I tracked recent 1000 blocks from block `842457` to `843457`

~61% of transactions in the last 1000 blocks were confirmed in a cluster size > 1.
~38% of transactions in the last 1000 blocks were confirmed in a cluster size 1

Transactions: 3974143
Cluster size 1 transactions: 1516505
Approximate percenta
...
💬 BrandonOdiwuor commented on pull request "Add used balance to overview page":
(https://github.com/bitcoin-core/gui/pull/775#issuecomment-2113012373)
@hebasto Sorry I will be updating the PR description

There was a change in implementation. I created a different PR (https://github.com/bitcoin/bitcoin/pull/29062) based on the recommendation to refactor the `getbalance()` function which touches the bitcoin repo. trying to get it merged so that I can update this PR
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2113026149)
Thanks for review @rkrux
- I addressed all your comments
- Force pushed from c12a677cc250608171bc4f6311095b60ba24abab to 2563305c0aef3975a6321911db7e0f2a245486de
[compare diff](https://github.com/bitcoin/bitcoin/compare/c12a677cc250608171bc4f6311095b60ba24abab..2563305c0aef3975a6321911db7e0f2a245486de)
💬 hebasto commented on pull request "Add used balance to overview page":
(https://github.com/bitcoin-core/gui/pull/775#issuecomment-2113028202)
> There was a change in implementation. I created a different PR ([bitcoin/bitcoin#29062](https://github.com/bitcoin/bitcoin/pull/29062)) based on the recommendation to refactor the `getbalance()` function which touches the bitcoin repo. trying to get it merged so that I can update this PR

Okay. Until then, marking this PR as a draft.
📝 hebasto converted_to_draft a pull request: "Add used balance to overview page"
(https://github.com/bitcoin-core/gui/pull/775)
**Second part:** Fixes https://github.com/bitcoin-core/gui/issues/769

Add used balance to the overview page for wallets with the avoid_reuse flag enabled

### Prerequsite:

- **Part one (should be merged first)**: https://github.com/bitcoin/bitcoin/pull/28776

overview page when avoid_reuse is enabled
![Screenshot from 2023-11-02 18-10-06](https://github.com/bitcoin-core/gui/assets/15610188/825a29f9-0558-4957-a079-1cb707421986)

overview page when avoid_reuse is not enabled
![Screen
...
💬 jonatack commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2113028468)
> * Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/26990#pullrequestreview-1860423961) provided by @jonatack regarding first commit [6898219](https://github.com/bitcoin/bitcoin/commit/6898219043f42ae42e4703c549e656f56d276d13).
>
> * Updated second commit [cf7dd35](https://github.com/bitcoin/bitcoin/commit/cf7dd3564a3ace4153a32930f36bd78432b59097) following some directions [given](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1483546926) by @jonatack and [disc
...
🤔 BrandonOdiwuor reviewed a pull request: "rpc: introduce getversion RPC"
(https://github.com/bitcoin/bitcoin/pull/30112#pullrequestreview-2058555562)
Concept ACK
💬 stickies-v commented on pull request "rpc: Remove index-based Arg accessor":
(https://github.com/bitcoin/bitcoin/pull/29997#issuecomment-2113031374)
re-ACK fadb3eb57b4d207a678067b89caa45abf1f93702, addressed doc nits
💬 jonatack commented on issue "Apparently CJDNS network does not work with Tor on mainnet.":
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2113032656)
> I could work on this.

Hi @pinheadmz, was a pull opened for this/any update? (thanks!)
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601902513)
This is already thread safe due to its internal locks. If its only user is going to protect it anyway, the internal locking should go away, no?

> (not done in this PR) m_orphanage doesn't need its own lock anymore

Ah. Perhaps add `mutable Mutex m_mutex; // TODO: this lock is obsoleted by m_tx_download_mutex` in txorphanage.h
🤔 ajtowns reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2058441253)
I'm not sure how to be confident that the `UpdateBlockTipSync` / `hashRecentRejectsChainTip` would be correct and didn't miss an edge case. Rest looks good to me.
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601944049)
Doesn't this also need to be called from `InvalidateBlock()`, potentially each time the tip is updated?
💬 ajtowns commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1601912655)
Do `AssertLockNotHeld` before the `AssertLockHeld` calls?
💬 hebasto commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2113040073)
This PR still have a couple of unaddressed comments: https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998 and https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350.

@jadijadi Are you still working on this?
💬 brunoerg commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2113047528)
Concept ACK
💬 pinheadmz commented on issue "Apparently CJDNS network does not work with Tor on mainnet.":
(https://github.com/bitcoin/bitcoin/issues/24450#issuecomment-2113064951)
No PR yet but I am looking into which solution is best. Any thoughts ?
💬 ismaelsadeeq commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2113066401)
re-ACK 186a00e64490bb5b7b3c544346ae047ad1a66696