Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#issuecomment-1655402712)
review ACK ecfe507e07e9bdab210e04ebd3fc3b8ae9d6a094

Didn't test anything nor compile
👍 TheCharlatan approved a pull request: "test: Add SyncWithValidationInterfaceQueue to mockscheduler RPC"
(https://github.com/bitcoin/bitcoin/pull/28118#pullrequestreview-1551760387)
ACK fabef121b0cdfac6ec1985f6c08c5685a886ba5a

Seems like the correct thing to do here anyway. Also reproduced the `feature_fee_estimation` behavior change.
👍 TheCharlatan approved a pull request: "refactor: Remove unused raw-pointer read helper from univalue"
(https://github.com/bitcoin/bitcoin/pull/28168#pullrequestreview-1551784192)
tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8
💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277370453)
![Fuzz inputs until crash](https://github.com/bitcoin/bitcoin/assets/6399679/d8e5e3ca-f5be-45e0-8803-8f8950690624)


Funny how `-only_ascii=1` performs worse than `-only_ascii=0`.
💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277371055)
(or at least, not significantly better)
💬 darosior commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#discussion_r1277371969)
Maybe the target returning early on non-ASCII has basically the same effect?
💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1655477388)
`-mutate_depth=3` seems to be the best so far:

![Fuzz inputs until crash](https://github.com/bitcoin/bitcoin/assets/6399679/4c038738-ca3f-4c93-9003-dddfda972914)
💬 hebasto commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#discussion_r1277398254)
Done.
💬 hebasto commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1655483540)
> Can you remove the Windows Cirrus task here, as it will need to be removed either way?

Done.

> Maybe also print the `GITHUB_TOKEN` permissions at every start of the task, just to double check they are read-only?

They are printed by default. To see them, in https://github.com/hebasto/bitcoin/actions/runs/5685393993/job/15410187833, one needs to expand "Set up job", then expand "GIHUB_TOKEN Permissions".
💬 MarcoFalke commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1655488587)
Ok, so I guess someone needs to enable Actions in both orgs for this to proceed.
⚠️ Sjors opened an issue: "test: 999 of 999 multisig"
(https://github.com/bitcoin/bitcoin/issues/28179)
### Motivation

The stack size limit of 1000 [confuses even those who designed it](https://bitcoin.stackexchange.com/questions/117317/how-can-i-create-a-multi-sign-for-a-large-number-of-users/117320?noredirect=1#comment134168_117320).

It would be useful to have a test to demonstrate that a 999 of 999 Taproot multisig is possible: `multi_a(999, P1, …, P999)`. Especially to make sure our wallet can spend from it and doesn't allow a bigger one (no 999 of 1000 and no 1 of 1000).

### Possible sol
...
💬 MarcoFalke commented on pull request "ci: Run Windows native task on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28173#issuecomment-1655489674)
In the meantime it may be good to do 10 runs and then check how many of them fail
💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1655491023)
I still have to rebase this, will do soon(tm).
🚀 fanquake merged a pull request: "refactor: Remove unused raw-pointer read helper from univalue"
(https://github.com/bitcoin/bitcoin/pull/28168)
💬 Sjors commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1277413296)
Should we disable the adding of new inputs when `reduce_output` is set? If someone really intends to do that, they should probably use `outputs`.
🤔 stickies-v reviewed a pull request: "docs: Rewrite README to make it more appealing"
(https://github.com/bitcoin/bitcoin/pull/28174#pullrequestreview-1551889123)
NACK.

I think documentation needs to be as concise and simple as possible, but I feel like the current version does a good job at that already. The current language is factual and not more informal than it needs to be.

In a world of overmarketed crypto projects, I think maintaining sober documentation is a nice differentiation.
💬 stickies-v commented on pull request "docs: Rewrite README to make it more appealing":
(https://github.com/bitcoin/bitcoin/pull/28174#discussion_r1277416439)
I don't like this current rewording, but I think I agree with your intent of adding some more information of what the Bitcoin Core project stands for, e.g. highly reviewed, robust and stable code, one-stop-shop for running a node and wallet to fully interface with the Bitcoin Network, ...
⚠️ Sjors opened an issue: "Unsafe reduce_output when new coins are added"
(https://github.com/bitcoin/bitcoin/issues/28180)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

See inline discussion at https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1269177446

@whitslack:

> Consider the case where I am paying someone on chain under the stipulation that _they_ will eat the mining fee. (Thus, I specify _their_ address in `subtractfeefrom` when I'm constructing my transaction.) Later, they tell me that they can't wait for confirmation any longer and
...
💬 Sjors commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1277417319)
Made a separate issue to track this: #28180
👍 TheCharlatan approved a pull request: "ci: Keep system env vars as-is (bugfix)"
(https://github.com/bitcoin/bitcoin/pull/28138#pullrequestreview-1551904341)
lgtm ACK fabc04a4d96c4fe70e60d365aa28031d149094f3