Bitcoin Core Github
42 subscribers
125K links
Download Telegram
achow101 closed a pull request: "wallet: add codex32 argument to addhdkey"
(https://github.com/bitcoin/bitcoin/pull/32652)
💬 achow101 commented on pull request "wallet: add codex32 argument to addhdkey":
(https://github.com/bitcoin/bitcoin/pull/32652#issuecomment-3433012113)
Concept NACK
💬 instagibbs commented on pull request "test: Add `leaf_version` parameter to `taproot_tree_helper()`":
(https://github.com/bitcoin/bitcoin/pull/29371#issuecomment-3433012786)
> Previously the leaf version was hard coded, because we only currently support 1 leaf version.

Unless I'm missing something, this is untrue? https://github.com/bitcoin/bitcoin/blob/7d27af98c7cf858b5ab5a02e64f89a857cc53172/test/functional/test_framework/script.py#L870

https://github.com/bitcoin/bitcoin/blob/7d27af98c7cf858b5ab5a02e64f89a857cc53172/test/functional/feature_taproot.py#L1683

Can you explain why this is insufficient for usage?
💬 l0rinc commented on pull request "test: set number of RPC server threads to 2":
(https://github.com/bitcoin/bitcoin/pull/33679#issuecomment-3433021242)
Similar to https://github.com/bitcoin/bitcoin/pull/33485, ran the tests locally, seems fine, change makes sense.
ACK e9cd45e3d3c7592265ebf67387090b3df1501df4
💬 instagibbs commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-3433022832)
(since I was tagged)

I don't think this PR makes sense stand-alone, but should definitely be considered if any sort of annex relay is to be considered standard in some future.
💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-3433028838)
cc @achow101
💬 roconnor-blockstream commented on pull request "wallet: add codex32 argument to addhdkey":
(https://github.com/bitcoin/bitcoin/pull/32652#issuecomment-3433037188)
Can you elaborate?
💬 dergoegge commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3433037258)
Concept ACK
💬 maflcko commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-3433041896)
So I guess it can be closed for now?
💬 achow101 commented on pull request "build: Explicitly set Qt's `AUTO{MOC,RCC,UIC}` property per target":
(https://github.com/bitcoin/bitcoin/pull/32951#issuecomment-3433042167)
Are you still working on this?
💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3433062198)
> Actually it can reallocate on every size exhaustion, i.e. every resize.
I don't like the new reuse, that can actually be slower than before because now the code paths depend on each other.
Wouldn't reserving the vectors help here instead?

You are referring to iteration-to-iteration coupling? Yes, the iterations depend on each other, however in practice we still allocate less than before, so there should not be a slowdown. Are you suggesting something like:
```cpp
witnessprogram.reserve(
...
💬 achow101 commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-3433062796)
Are you still working on this?
💬 stickies-v commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#discussion_r2452533650)
The next sentence seems like a more logical place to put this.

```suggestion
- It's preferable to avoid changing an RPC in a backward-incompatible manner, but in that case, add an associated `-deprecatedrpc=` option to retain previous RPC behavior during the deprecation period. Backward-incompatible changes include: data type changes (e.g. from `{"warnings":""}` to `{"warnings":[]}`, changing a value from a string to a number, etc.), logical meaning changes of a value, key name changes (e.g.
...
achow101 closed a pull request: "improve MallocUsage() accuracy"
(https://github.com/bitcoin/bitcoin/pull/28531)
💬 achow101 commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#issuecomment-3433075236)
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

Marking as up for grabs.

cc @l0rinc @sipa
💬 Eunovo commented on pull request "descriptors: taproot partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-3433084069)
Closing until bip is ready
Eunovo closed a pull request: "descriptors: taproot partial descriptors"
(https://github.com/bitcoin/bitcoin/pull/30243)
achow101 closed a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699)
💬 achow101 commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3433084095)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.

Closing due to lack of interest.
🤔 ryanofsky reviewed a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676#pullrequestreview-3366446043)
Code review 0e0cf1822416956b6853e593517f4a9fab157a62. This looks very good and I did not see any issues at a quick glance. I think I might suggest shortening interruptWaitNext to interruptWait, but no strong opinion.