Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 Sjors commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493842596)
> It was also intentionally selected to support 32-bit mode.

That's useful to know.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853928443)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853938164)
Added.
🤔 jamesob reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2454451702)
Feedback addressed. Thanks for all review.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853917742)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853918927)
Good point, removed. I forgot that I had a main-chain check in there. I debated about whether this call would be useful for checking balances on orphaned tips, but probably better to just restrict usage to the main chain to avoid confusion.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853914205)
Fixed.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853916616)
I'm going to leave this as-is.

I think partial results would be weird. I like the current behavior that it will succeed if possible but fail if results would be missing; I think any kind of requirement about running pruned or not would impair cases for which the descriptors the user cares about are on the "right side" of the prune cliff.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853919432)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853914047)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853924116)
Done.
💬 vasild commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2493853269)
@theStack, yes, it was that one, to `11.22.33.44`, thanks!

Lets keep the focus here on enforcing this in the CI, rather than individual tests that do that.

I couldn't find the output of `iptables -j LOG` in the CI - it is nowhere to be found in `/var/log` nor in `dmesg` output even though `iptables -v -x -n -L` shows some packets matched. Any help would be welcome!

Using `tcpdump(1)` is another approach that got me further: https://github.com/bitcoin/bitcoin/pull/31349 and it detects in
...
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1853983766)
The depends sources cache still seems to contain unversioned variants of `CMakeLists.txt`, `ECMOptionalAddSubdirectory.cmake` and `QtTopLevelHelpers.cmake`: https://cirrus-ci.com/task/4792887920033792
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2493868479)
Nice. Conecpt ACK!
💬 Sjors commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493871800)
I updated the `waitTipChanged()` and `m_tip_block` doc.

@maflcko which if-guard do you mean?
💬 ryanofsky commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493880145)
> @maflcko which if-guard do you mean?

I think probably this one:

```c++
// Wait for genesis block to be processed
if (WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip() == nullptr)) {
```

Removing the `if()` might help verify that `m_tip_block` is set reliably
💬 maflcko commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493892510)
Also, the ZERO workaround could be removed completely, ensuring that any interfaces will only be spun up after that point in init?
👍 ryanofsky approved a pull request: "Drop script_pub_key arg from createNewBlock"
(https://github.com/bitcoin/bitcoin/pull/31318#pullrequestreview-2454619280)
Code review ACK 027ce3e9634b2f47c508899942d05690572de516. Since last review, this was rebased to avoid conflicts, and option is now a plain cscript instead of an optional\<cscript>, and the commits are split up and there are some minor changes in tests (renames, whitespace, dropping redundant assignments. This is pretty easy to review now that the main commit is so small and changes are split up
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1854018677)
Thanks, cleared the machine