Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 Sarsilmazxx02 opened a pull request: "curl -sSL "https://mempool.space/api/mempool/recent""
(https://github.com/bitcoin/bitcoin/pull/30874)

```python
[
{
txid: "4b93c138293a7e3dfea6f0a63d944890b5ba571b03cc22d8c66995535e90dce8",
fee: 18277,
vsize: 2585,
value: 4972029
},
...
]
```




<!--
*** 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 mot
...
💬 Sarsilmazxx02 commented on pull request "curl -sSL "https://mempool.space/api/mempool/recent"":
(https://github.com/bitcoin/bitcoin/pull/30874#issuecomment-2344598401)
```c++
diff --git a/.github/workflows/c-cpp.yml b/.github/workflows/c-cpp.yml
new file mode 100644
index 0000000000000..fbf32ec92d3bf
--- /dev/null
+++ b/.github/workflows/c-cpp.yml
@@ -0,0 +1,23 @@
+name: C/C++ CI
+
+on:
+ push:
+ branches: [ "master" ]
+ pull_request:
+ branches: [ "master" ]
+
+jobs:
+ build:
+
+ runs-on: ubuntu-latest
+
+ steps:
+ - uses: actions/checkout@v4
+ - name: configure
+ run: ./configure
+ - name: make
+ run
...
💬 Sarsilmazxx02 commented on pull request "assumeutxo state and locking cleanup":
(https://github.com/bitcoin/bitcoin/pull/28608#issuecomment-2344612336)
```cmake
diff --git a/.github/workflows/c-cpp.yml b/.github/workflows/c-cpp.yml
new file mode 100644
index 0000000000000..fbf32ec92d3bf
--- /dev/null
+++ b/.github/workflows/c-cpp.yml
@@ -0,0 +1,23 @@
+name: C/C++ CI
+
+on:
+ push:
+ branches: [ "master" ]
+ pull_request:
+ branches: [ "master" ]
+
+jobs:
+ build:
+
+ runs-on: ubuntu-latest
+
+ steps:
+ - uses: actions/checkout@v4
+ - name: configure
+ run: ./configure
+ - name: make
+ r
...
💬 l0rinc commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1755545984)
nit: I've also removed `IF_CHECK_FAILED` in [`26cbe1e` (#30732)](https://github.com/bitcoin/bitcoin/pull/30732/commits/26cbe1e95d7960c6b4de19b77240b62f4f27d73b), consider readjusting the comment alignment as well
💬 l0rinc commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1755550025)
It seemed to me that `SOURCE` is also unused here, see: https://github.com/bitcoin/bitcoin/commit/031ac7f366263bacab152aa94a51025c582b1191#diff-d7f6f763fed50046f1c8a5dcca088fabb6b8c3662a55516f08fc0d0ba24829e3L45
📝 fanquake locked a pull request: "curl -sSL "https://mempool.space/api/mempool/recent""
(https://github.com/bitcoin/bitcoin/pull/30874)

```python
[
{
txid: "4b93c138293a7e3dfea6f0a63d944890b5ba571b03cc22d8c66995535e90dce8",
fee: 18277,
vsize: 2585,
value: 4972029
},
...
]
```




<!--
*** 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 mot
...
🤔 hodlinator reviewed a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2298052654)
Reviewed fa26462e95291652b4021d91b014655f678149e8

`git range-diff master fa7819b fa26462`

- Moved independent commit to the beginning, thanks!
- Added some comments.

I really like that this is being worked upon and want to help us find the best trade-offs in settling on a solution.
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755206069)
> but that's not what this does, that's below this condition.

Ah, yes, mixed them up. Still feels okay to ignore anything up to a '%' that way.

But I do also like l0rinc's approach in this thread https://github.com/bitcoin/bitcoin/pull/30546/files#r1754649705, although extra state tends to create room for more bugs.
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755352516)
Duplicates
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755589971)
#### A) Interlacing `tfm::format`

Partially in response to @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755013263)...

> Potentially we could even extend the test to compare tinyformat behavior with ConstevalFormat behavior to make things even clearer, but just having a comment to clarify how these cases are treated by tinyformat would be very helpful.

...and partially out of my own curiosity about how `tfm::format` really behaves - I added `tfm::form
...
💬 maflcko commented on pull request "http: Use 'starts_with' for matching URI prefix":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2344697879)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 hodlinator commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1755611792)
> Pushing has a specific meaning in the context of Bitcoin Script (it's an opcode that pushes data onto the stack), and that's not what's happening here so that would be very confusing.

The new pair of *private* functions together implement `OP_PUSHDATA`, and are *only used* in that context, that's the reason I recommended the naming.
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755612640)
Thanks, I'll fix it up in the next follow-up, or if I have to re-push here, to avoid invalidating review over a harmless test style-nit.
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755647995)
> While I agree with maflcko that [keeping functionality hard to use outside of the intended context](https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1752494409) has value, keeping tests straightforward is more important. If you prefer...

Thank you for the diff. However, I think that real code should be designed without taking tests into account (apart from being testable). Tests should always follow real code, not the other way round. I am sure the verbosity in the test can be redu
...
👍 TheCharlatan approved a pull request: "build: Add more cmake presets"
(https://github.com/bitcoin/bitcoin/pull/30871#pullrequestreview-2298717809)
ACK f15e817811e328423ea489870ead089128a6ef8c
👍 hodlinator approved a pull request: "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors"
(https://github.com/bitcoin/bitcoin/pull/30765#pullrequestreview-2298723649)
re-ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7

`git range-diff master e6a5ab7 5e190cd`

- [Disagree](https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1755611792) with `AppendData`/`AppendDataSize` names over `PushData`/`PushDataSize`, but will let it go.
- Nice switch back from pointer arithmetic to `span` in `AppendData`.
- Touched up commit messages per my latest review, cheers!
- Implements legacy `operator<<` in terms of the other. I agree with l0rinc's reservations against
...
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755702207)
> I think that real code should be designed without taking tests into account (apart from being testable). Tests should always follow real code, not the other way round.

Making the function return a value allowed for C) dropped negatives (and less runtime testing). But I can try to provide a new diff bringing them back and without changing *string.h*. My main point remains adding the interlaced tinyformat tests to prove parity.

> The diff you shared also changes `util::hex_literals::detail
...