Bitcoin Core Github
43 subscribers
122K links
Download Telegram
fanquake closed a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29096)
📝 fanquake locked a pull request: "Rename SECURITY.md to SECURITY.md."
(https://github.com/bitcoin/bitcoin/pull/29096)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713566)
> It would just be a one-line function to hold the code you already wrote in two places:

oh got it now.

> I wonder if this can be generalized to pass test-only options (along with a helper HasTestOption, similar to IsDeprecatedRPCEnabled?

good idea - liked how we can make sure users don't accidentally use these test-only options in their config file.
done.
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713673)
> I think it would be ideal if we could avoid the addpeeraddress test leaking into getaddrmaninfo and getrawaddrman tests.

it would be cleaner to not need to know the existing context of addrman when dealing with getaddrmaninfo/getrawaddrman tests. both the options sound good to me!

> The easiest way of producing a collision is by adding more and more addresses to the tried table (as we do in the unit test). This would leave us with quite a few additional addresses already in the tried tab
...
💬 stratospher commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428713695)
done.
⚠️ helpau opened an issue: "Connection between nodes on the PC interfaces doesn't work"
(https://github.com/bitcoin/bitcoin/issues/29097)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The nodes haven't started synchronising, User-Agent are doubled
![image](https://github.com/bitcoin/bitcoin/assets/30323047/046760e0-8148-4cd0-a2a3-fdaf92a4806c)


### Expected behaviour

The nodes have started synchronising, User-Agents are different

### Steps to reproduce

Preconditions: Bitcoin Core 25.1(reference node, partially synchronized) and self-compiled Bitcoin Core(from mast
...
💬 helpau commented on issue "Connection between nodes on the PC interfaces doesn't work":
(https://github.com/bitcoin/bitcoin/issues/29097#issuecomment-1858753241)
Probably related #21747 #22559
⚠️ Sjors opened an issue: "Performance decrease after tapscript miniscript"
(https://github.com/bitcoin/bitcoin/issues/29098)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

@eriknylund noticed while testing large multisigs that after 4f473ea515bc77b9138323dab8a741c063d32e8f `sendtoaddress` causes a timeout for much smaller (though still rather large) multisigs than before.

https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1428484311

### Expected behaviour

Not sure, perhaps the performance decrease is justified. Ideally it would not slow down.


...
💬 Sjors commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1428740238)
Great find! Let's track this in a separate issue: #29098
💬 ajtowns commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1858768140)
> Right, seems fine to keep the assert. I was just thinking that it would be nice to drop it if the compiler can prove it isn't needed. Currently that only works for std::array or `[]` types. However, it doesn't seem to work out of the box for array wrappers, like uint256, whose extent is known at compile time.

So, number one, I think you need an helper to implicitly convert a uint256 to a constant-extent span or you'll have to be explicit everywhere:

```c++
class base_blob
{
...
c
...
ajtowns closed a pull request: "NOMERGE UFG Default permitbaremultisig=0"
(https://github.com/bitcoin/bitcoin/pull/29093)
💬 ajtowns commented on pull request "NOMERGE UFG Default permitbaremultisig=0":
(https://github.com/bitcoin/bitcoin/pull/29093#issuecomment-1858768492)
Okay closing this as up for grabs; should be easy if anyone wants to pick it up.
💬 0xB10C commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1428780378)
> > The easiest way of producing a collision is by adding more and more addresses to the tried table (as we do in the unit test). This would leave us with quite a few additional addresses already in the tried table.
>
> these collisions would only affect future tests added after getaddrmaninfo/getrawaddrman tests though.

We'd need to clean up after we found a collision.
💬 maflcko commented on issue "Flaky `wallet_transactiontime_rescan.py --legacy-wallet` functional test":
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1858812580)
Could bump the timeout inside this test to fix it?
💬 TheCharlatan commented on pull request "doc: Rework guix docs after 1.4 release":
(https://github.com/bitcoin/bitcoin/pull/28962#discussion_r1428813691)
Not really related to this patch, but why is this suggesting to turn networking on and off? Seems like an efficient way to lock yourself out of a box.
💬 TheCharlatan commented on pull request "doc: Rework guix docs after 1.4 release":
(https://github.com/bitcoin/bitcoin/pull/28962#discussion_r1428810054)
`s/packages/packaged/`
📝 Retropex reopened a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217)
The default activation of the `permitbaremultisig=0` option proposes an enhancement for the Bitcoin network. By refusing non-P2SH multisignature transactions from the outset, this modification would contribute to reducing spam attempts and maintaining a healthy decentralization by discouraging undesirable activities.
💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1428842615)
> Whatever argument is given to -datadir=, always make (if necessary) and use a subdirectory (within the specified directory) called test_bitcoin. That way, even if the user specifies their real data directory, all the test stuff will be within this subdirectory. (It's similar to how there already are subdirectories for testnet, signet, and regtest.)

Sounds good. s/test_bitcoin/tests (we don't use "signet_bitcoin", "testnet_bitcoin", "regtest_bitcoin").

------------

Also, probably somet
...
💬 fanquake commented on pull request "NOMERGE UFG Default permitbaremultisig=0":
(https://github.com/bitcoin/bitcoin/pull/29093#issuecomment-1858860418)
Removed up for grabs given #28217 has been reopened.
👋 Retropex's pull request is ready for review: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217)