Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216982231)
nit: could do IWYU I guess, even though this wasn't followed that well in the index cpp files.

```
#include <dbwrapper.h>
#include <interfaces/types.h>
```
💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216992529)
Also, very minor naming nit: File could be called `db_key.h` since we have other kinds of keys but I don't feel too strongly about it.
💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216991906)
Aside from the documentation moving necessary mentioned above maybe there could be 1-2 general sentences here that this deals with the hash/height keys and that it's mostly relevant to indexes that use these.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217023785)
Ok, will take @maflcko's suggestion if I have to push again.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217028864)
Did that before, but @hodlinator pointed out that the previous version made more sense since it's storing obfuscation key's database key: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2068663871.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217024259)
> The simplest way to write this would be:
> m_obfuscation = Obfuscation{FastRandomContext{}.randbytes<Obfuscation::KEY_SIZE>()};
> Write(OBFUSCATION_KEY_KEY, m_obfuscation);

I don't think so, `Write` needs `m_obfuscation`, so it would obfuscate the key itself if we assign it before the write, try `build/bin/test_bitcoin --run_test=dbwrapper_tests`.
Seems the comment I added still isn't helping, how do I rephrase it to make this part obvious?

```C++
const Obfuscation new_key{FastRandom
...
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217039648)
Not sure I understand why this is better, we have the xor instructions spread to multiple places now - apply_random_xor_chunks & inline `original ^ key_bytes`. What do others think?
🤔 l0rinc reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-3034969188)
Thanks for the review.
Since the PR is in review for almost a year now, and since the remaining comments seem like they can potentially be done in follow-ups as well, I'd prefer getting this over the finish line soon.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217034784)
Sure, if I push again, I'll consider these nits
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217030868)
If you insist, I'll do it if I have to push again.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217033100)
Can you please point me to a documentation or code which demonstrates that?
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2217040108)
Sure, I'll merge those commits next time I push
💬 l0rinc commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3091278861)
Had to rebase after https://github.com/bitcoin/bitcoin/commit/067365d2a8a421a074bb54394118beccb3f775c2#diff-e6100361fa0e9e25478f808ca084e5f681d4dddbbee7b3bea0f9d5bcd29db3aaR39-R563, `src/txorphanage.cpp` was moved to `src/node/txorphanage.cpp`, the change is exactly the same otherwise, would appreciate re-reviews.
💬 achow101 commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-3091292177)
ACK 248b6a27c351690d3596711cc36b8102977adeab
💬 lifofifoX commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3091388547)
> A short while ago I sent an RBF with a 0.5 sat/vB fee, waited 30 minutes, and it still hadn’t reached mempool.space’s node

I believe that's because mempool.space nodes did not lower `incrementalrelayfee` through [config](https://github.com/mempool/mempool/blob/master/production/bitcoin.conf#L23C1-L23C14).
📝 b-l-u-e opened a pull request: "rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures"
(https://github.com/bitcoin/bitcoin/pull/33014)
## Summary
Fixes #32849 - Replace `CHECK_NONFATAL` with proper error handling in `descriptorprocesspsbt` RPC to prevent internal bug assertions when encountering invalid Schnorr signatures.

## Problem
When `descriptorprocesspsbt` encounters a PSBT with invalid signatures (e.g., invalid Schnorr signatures with SIGHASH_SINGLE | ANYONECANPAY flags), it triggers an internal bug assertion instead of returning a user-friendly error message.

## Solution
Replace the `CHECK_NONFATAL(FinalizeAndE
...
🚀 achow101 merged a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144)
💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3091891676)
> Minified this is:

I don't think you need the `git fetch; git checkout` here -- you get the same error with that command (dropping the `/bitcoin`) on master afaics?
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-3091927892)
Thanks a lot @maflcko, @ryanofsky, @hodlinator and @achow101 for all the reviews and reproducers and suggestions!
💬 stwenhao commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3091997169)
> you can't force people to pay higher fees unless it's a consensus rule

I think even if consensus rules would dictate fees, then it would still be possible to bypass them. For example: users can sign a given transaction, only if the next one would send them their coins back. Which means, that if free transactions would be consensus rule, then users would use additional anchor-like outputs to pay to miners. And if 100% fee transactions would be required by consensus, then miners could still s
...
💬 ArmchairCryptologist commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092171983)
> When we have 1 sat/vB, then miners are guaranteed to get at least 0.01 BTC in fees per block

This is only correct if there are enough 1+ sat/vB transactions to fill the block. Miners are free to set their own minimum fee policy, and if some "fee market bidders" (transactors) are unwilling to pay 1 sat/vB and some "fee market buyers" (miners) are willing to mine at less than 1 sat/vB, miners who aren't willing to mine at less than 1 sat/vB blocks would be mining less-than-full blocks instead
...