Bitcoin Core Github
44 subscribers
121K links
Download Telegram
fanquake closed an issue: "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions"
(https://github.com/bitcoin/bitcoin/issues/29137)
fanquake closed an issue: "Bitcoin: Operation Sweep - an L1 upgrade proposition"
(https://github.com/bitcoin/bitcoin/issues/29138)
💬 theStack commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1435589223)
Good catch, done.
💬 theStack commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1868285388)
Thanks for the reviews! I've tackled the [missing header include](https://github.com/bitcoin/bitcoin/pull/28455#discussion_r1434172881) and the two more instances where the refactoring can be done ([script_standard_Solver_failure](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard_tests.cpp#L141) and [script_standard_ExtractDestination](https://github.com/bitcoin/bitcoin/blob/7524fcff8625f0197be6cf84df285c39fcd5d6b6/src/test/script_standard
...
💬 ryanofsky commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572)
Since this PR is adding the `createwalletdescriptor` method, maybe this is a good place to list some ways it could be extended in the future:

- Probably it would be good not to require wallet to be unlocked when dealing with public keys. Currently the specified `hdkey` is unencrypted and reencrypted, but this shouldn't be necessary because the key is already in the wallet. (The only reason this seems to happen now is to copy the key, because internally we store keys in a slightly denormalized
...
💬 derekm commented on issue "Spam Filter Code in Tx Relay & Mempool Acceptance Ignores OFAC SDN Sanctions":
(https://github.com/bitcoin/bitcoin/issues/29137#issuecomment-1868306547)
@fanquake -- Bitcoin Core's spam filter code already has this `Solver` utility which provides a `vSolutionsRet` output parameter that contains exactly the data that needs to be matched against OFAC SDN sactions against XBT: https://github.com/bitcoin/bitcoin/blob/master/src/script/solver.cpp#L140

I am working on a PR to implement enforcement of these Sanctions, which enforcement is required of every U.S. person or foreign persons working for U.S. persons.

With this Issue being posted to th
...
💬 ryanofsky commented on pull request "rpc: add path to gethdkey":
(https://github.com/bitcoin/bitcoin/pull/22341#issuecomment-1868311232)
> Initially I'll make a separate PR. If #26728 is closed in favor of the new approach in #29130 then this PR can be closed too.

It seems like you could just rebase this on top of #29130, since the description of the PR and commits aren't going to change that much, they will just be based on #29130 instead of #26728. Either way seems fine, though.
⚠️ GregTonoski opened an issue: "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf"
(https://github.com/bitcoin/bitcoin/issues/29139)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

There is the bitcoin.conf file in the bitcoin-26.0 directory. It's not obvious that the configuration from the file is not used by Bitcoin Core.

Severity: minor (cosmetic issue).

### Expected behaviour

There isn't the same filename (bitcoin.conf) used for the description (./bitcoin.conf) and actual configuration (./datadir/bitcoin.conf).

### Steps to reproduce

Extract archive/insta
...
🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1795448339)
For the sake of completeness and to avoid potential issues, have expanded the test coverage to exercise the previously mentioned "concurrent database transactions deadlock" behavior. All yours: https://github.com/furszy/bitcoin-core/commit/a80cc56f0d1bdc82e9458f957db03c08620a6dd7. It verifies that the database handler cannot commit/abort changes occurring in a different database handler.

Also, while was working on this, discovered another deadlocking cause.. test https://github.com/furszy/bit
...
💬 furszy commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1435632201)
In 689ed43c2:

Pulling mistake here. Introduced another `DatabaseBatch`. Just need one.
💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1435666435)
This wouldn't trigger the warning if the median is equal to `std::numeric_limits<int64_t>::min()`
📝 luke-jr opened a pull request: "Bugfix: RPC: Check for blank rpcauth on a per-param basis"
(https://github.com/bitcoin/bitcoin/pull/29141)
Without this, a blank rpcauth will error if there is a non-blank following it, and a non-blank rpcauth will get ignored if the last one is blank

Includes test updates to detect issues

(It might make sense to support `-norpcauth`/`-rpcauth=0` to disable all defined `-rpcauth` options, but that isn't currently supported, so out of scope here)
💬 luke-jr commented on pull request "Bugfix: RPC: Check for blank rpcauth on a per-param basis":
(https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1868353002)
I'm not entirely sure what the expected behaviour here is...
💬 pinheadmz commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1868425688)
Are you referring to the file https://github.com/bitcoin/bitcoin/blob/26.x/share/examples/bitcoin.conf ?

Which includes the preface:

```
## All possible configuration options are provided. To use, copy this file
## to your data directory (default or specified by -datadir), uncomment
## options you would like to change, and save the file.
```

`bitcoin.conf` usage is also covered in https://github.com/bitcoin/bitcoin/blob/master/doc/bitcoin-conf.md and again in https://github.com/bitc
...
💬 kashifs commented on pull request "Add Gutter Guard Selector":
(https://github.com/bitcoin/bitcoin/pull/28977#issuecomment-1868501864)
tested ACK [4a94fc8](https://github.com/bitcoin/bitcoin/pull/28977/commits/4a94fc8d827499d120d38002406da8f6c6c731ac)

I read through the conversation, understood the problem and use case for this solution, compiled from source, wrote tests similar to those included
💬 theStack commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1435835891)
more-pythonic-nit:
```suggestion
kwargs['services'] = kwargs.get('services', P2P_SERVICES) | NODE_P2P_V2
```
(also in the `add_outbound_p2p_connection` method below)
👍 theStack approved a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1795584001)
ACK ad0ae3d2128d04ff4f62a4bf612286d153dc314b
💬 theStack commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1435840695)
nit: IIUC, `b""` is also returned if the decrypted package content is empty. As call-sites currently use
the length integer return value (0) to check for the "only part of packet is received" condition, this isn't a problem, but maybe the description can be improved to avoid confusion.
💬 1440000bytes commented on pull request "Compressed Bitcoin Transactions":
(https://github.com/bitcoin/bitcoin/pull/29134#issuecomment-1868547068)
Concept ACK

25-50% savings look good and compressed raw transactions can help in some cases as mentioned in the PR description.

Related Q&A: https://bitcoin.stackexchange.com/questions/98886/compress-transaction-hex-string (still wont fit in one text message but will need less text messages)
🤔 furszy reviewed a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1795601906)
Sorry for the delay @naumenkogs. 've been on #28170 before returning here. All good now, it is ready to go.

> The code looks alright, but i'd still rather have [this](https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1557211930) addressed first. It looks like even #28170 doesn't directly handle this, but rather prepare the codebase for another PR handling it?

The concern was:
All connections are from limited peers, we are lagging behind and we no longer disconnect from them
...