Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🚀 achow101 merged a pull request: "ci: move ASan job to GitHub Actions from Cirrus CI"
(https://github.com/bitcoin/bitcoin/pull/30193)
💬 paplorinc commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643367751)
I wanted a value that's definitely oversized, to avoid being close to the threshold - but I think I managed to make it work, thanks for the hint, let me know if you'd like me to reduce the duplication in what I've pushed.
💬 achow101 commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2174342443)
ACK 0e0c422aedd4009ab34eca127e4904d15e81f5be

Not as familiar with locking in their area, so not super confident on the `cs_main` related changes, but the rest (removing other mutexes and consolidating under `m_tx_download_mutex`) looks fine.
💬 rkrux commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1643385636)
Understood now!
Thanks @achow101, this is very helpful.
💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1643388352)
I think that should be OK, assuming it doesn't happen to hit a case where there's a short-ID collision
💬 AngusP commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1643388594)
Sure, seems sensible to me
👍 rkrux approved a pull request: "test: fix MiniWallet script-path spend (missing parity bit in leaf version)"
(https://github.com/bitcoin/bitcoin/pull/30076#pullrequestreview-2123842586)
reACK [e4b0dab](https://github.com/bitcoin/bitcoin/pull/30076/commits/e4b0dabb2115dc74e9c5794ddca3822cd8301c72)

Applied the shared patch again and the following tests fail on master with the mentioned error. Though I've not been able to go through the corresponding control block C++ code yet :(

```
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: mandatory-script-verify-flag-failed (Witness program hash mismatch) (-26)
```

```
mempool_limi
...
💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643398546)
Is the sole reason for creating 10 tagged wallets to be thorough? Or is there any other reason as well?
💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1643399436)
i.e. to target the following assumption
> we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity.
💬 achow101 commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2174371181)
ACK 260f8da71a35232d859d7705861fc1a88bfbbe81
💬 rkrux commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643416859)
Nice to have: To reduce duplication by using lambda expressions and captures.
👍 rkrux approved a pull request: "test: Validate oversized transactions or without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2123875126)
reACK [fa9e680](https://github.com/bitcoin/bitcoin/pull/29862/commits/fa9e680b0ca3c401a33e9901e8732128113be85e)

Re-tested by running make and make check, both successful.
Mentioned couple suggestions but not blockers from my side, upto author.
💬 rkrux commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643413535)
Would be nice to add a comment explaining the presence of this constant 70 for future reference.
💬 prusnak commented on pull request "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic"":
(https://github.com/bitcoin/bitcoin/pull/30282#issuecomment-2174418333)
utACK
👍 prusnak approved a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282#pullrequestreview-2123899637)
utACK
💬 achow101 commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2174428338)
ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
🚀 achow101 merged a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058)
💬 achow101 commented on pull request "Release `LockData::dd_mutex` before calling `*_detected` functions":
(https://github.com/bitcoin/bitcoin/pull/26781#issuecomment-2174448016)
@hebasto Are you planning on addressing review feedback here?

***

> I think it would be better if it had another commit getting rid of StdMutex/StdLockGuard. Otherwise nothing is really taking advantage of the change.

I agree. The PR description sounded like this was going to be the ultimate change, but it does not appear to have actually been done.
🚀 achow101 merged a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)
💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-2174450450)
> > > ```diff
> > > --- a/test/functional/wallet_import_rescan.py
> > > +++ b/test/functional/wallet_import_rescan.py
> > > @@ -293,6 +293,7 @@ class ImportRescanTest(BitcoinTestFramework):
> > > add_to_wallet=False,
> > > inputs=[unspent_txid_map[variant.initial_txid]],
> > > outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}],
> > > + locktime=0,
> > > subtract_fee_from_outputs=[0]
> > >
...