Bitcoin Core Github
44 subscribers
121K links
Download Telegram
๐Ÿ’ฌ 1440000bytes commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2417770876)
By default, the RPC interface can only be accessed [locally](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#security). However, a user may want to keep the RPC local and enable the REST interface with [`rest=1`](url) to allow public access to available [endpoints](https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md).

This pull request adds a basic endpoint to the REST interface for broadcasting transactions. As long as Bitcoin Core supports the REST
...
๐Ÿ‘ pablomartin4btc approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2373584648)
re-ACK 15cfeebb47587af6ce7be72fd52c57a0483d86d2

Changes since my last review: removing the `-testdatadir` arg from `bench-bitcoin` and reusing it from the utils `setup_common`.
๐Ÿค” ariard reviewed a pull request: "validation: Improve input script check error reporting"
(https://github.com/bitcoin/bitcoin/pull/31097#pullrequestreview-2373597013)
utACK ad51775
๐Ÿ’ฌ ariard commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1803751039)
I was reading the comment just above this one i.e โ€œCheck whether the failure was caused by a non-mandatory script verification check, such as non-standard DER encodings or non-null dummy argumentsโ€.

I think the 2 examples given in this comment are actually โ€œmandatory-script-verify-flagโ€, i.e CONSENSUS and not NOT_STANDARD:
- DER encodings (BIP66 and `SCRIPT_VERIFY_DERSIG`), interpreter flag is set in `GetBlockScriptFlags` https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L237
...
๐Ÿ’ฌ ariard commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1803776359)
I think we should have further tests that a non-standard transaction which is consensus valid before an activation height, is considered as consensus invalid after the activation height.

E.g on regtest, the deployment height are always active unless overridden. I donโ€™t think we do that in invalid_txs.py` currently: https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_invalid_tx.py#L28
๐Ÿ‘ adamandrews1 approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2373745550)
ACK
๐Ÿ’ฌ l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#issuecomment-2418113705)
While the build likely hasn't decided how to handle casts, the benchmarks are looking quite promising already:

<details>
<summary>benchmark</summary>

hyperfine \
--runs 1 \
--export-json /mnt/my_storage/ibd_full-prune.json \
--parameter-list COMMIT 48cf3da636089873ba7280e0d5b22eb81811d194,0391844fa123a47ddf479e865f9eec63f28a4c6c \
--prepare 'rm -rf /mnt/my_storage/BitcoinData/* && git checkout {COMMIT} && git clean -fxd && git reset --hard && cmake -B build -DCMAKE_BUILD_TYPE=Release
...
๐Ÿ’ฌ hodlinator commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2418140042)
Another one! https://github.com/hodlinator/bitcoin/actions/runs/11368998912/job/31625398583
โš ๏ธ davemiketony opened an issue: "Building libsecp256k1 fails on OpenBSD 7.6"
(https://github.com/bitcoin/bitcoin/issues/31106)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Building libsecp256k1 by following the instructions located within `src/secp256k1` fails on OpenBSD 7.6 (the latest OpenBSD release).

The instructions fail during the autogen step with errors mentioning libtool.

### Expected behaviour

A successful build of a libsecp256k1 is possible by strictly following the instructions found within `src/secp256k1/README.md`.

### Steps to reproduce

...
๐Ÿ’ฌ fuleru commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2418275162)
Itโ€™s a bit scary. Currently, testnet4 block generation is controlled. https://mempool.space/testnet4/address/tb1q2dsc94zq40nwnz27w5rxljwllutnwjtlxk44fz
๐Ÿค” glitteryskye99 reviewed a pull request: "doc: update signet documentation related to build directories"
(https://github.com/bitcoin/bitcoin/pull/30996#pullrequestreview-2374014243)
Interesting
๐Ÿค” ariard reviewed a pull request: "validation: Improve input script check error reporting"
(https://github.com/bitcoin/bitcoin/pull/31097#pullrequestreview-2374018383)
Re-tested, getting the correct RPC string.
๐Ÿ’ฌ ariard commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1804025893)
```
diff --git a/src/validation.cpp b/src/validation.cpp
index fc61b14dc1..ecc1ab22dd 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1970,7 +1970,7 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
CScriptCheck check2(txdata.m_spent_outputs[i], tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata);
if (check2())
- return state.Invalid(TxValidationR
...
๐Ÿ’ฌ l0rinc commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#issuecomment-2418832550)
@martinus are you still working on this or should we review the cherry-picked changes in https://github.com/bitcoin/bitcoin/pull/31102 instead?
๐Ÿ‘ laanwj approved a pull request: "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues"
(https://github.com/bitcoin/bitcoin/pull/30349#pullrequestreview-2374468113)
ACK c49cc30b8185a50b4aa00cf705d313c8aa9482a3
๐Ÿ’ฌ martinus commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#issuecomment-2418867904)
Hi @l0rinc, sorry I most likely can't work on this or anything else in the forseeable future
โœ… fanquake closed a pull request: "memusage: let PoolResource keep track of all allocated/deallocated memory"
(https://github.com/bitcoin/bitcoin/pull/28939)
๐Ÿ’ฌ l0rinc commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#issuecomment-2418877072)
Thanks for the quick response, @andrewtoth cherry-picked your changes and we'll continue there
โœ… fanquake closed an issue: "doc: Fixup windows-cross build notes"
(https://github.com/bitcoin/bitcoin/issues/31090)
๐Ÿš€ fanquake merged a pull request: "doc: remove dependency install instructions from win docs"
(https://github.com/bitcoin/bitcoin/pull/31100)
๐Ÿ‘ laanwj approved a pull request: "doc: remove dependency install instructions from win docs"
(https://github.com/bitcoin/bitcoin/pull/31100#pullrequestreview-2374503030)
ACK 184f12c1542f6c53eb2bd9dfb08dfdd490e38846
i agree this is the better way to handle documentation, have as little redundancy as possible.
Also always found the reference to `build-windows.md` from the depends `README.md ` kind of surprising, so i'm glad it's removed.