💬 stickies-v commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416432329)
> Using RPC requires an authenticated connection.

Could you please elaborate on why RPC or authentication is not a feasible option for your use case?

🤔 stickies-v reviewed a pull request: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065#pullrequestreview-2372023562)
I'd prefer not extending the REST interface if we don't have to. This functionality is already well supported in the RPC, so this seems like purely a convenience endpoint?

I've asked for clarification in https://github.com/bitcoin/bitcoin/issues/31017, but until there's a good reason to add this I'm Concept NACK.

💬 glozow commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802861498)
This is what testmempoolaccept does

🤔 instagibbs reviewed a pull request: "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs"
(https://github.com/bitcoin/bitcoin/pull/24128#pullrequestreview-2372024041)
concept ACK, appears to adhere to the BIP on first glance

will do deeper review if tests are applied

💬 maflcko commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1802863026)
Either should be fine, because it just ends up in the debug log (not really for end-users). What about:

```cpp
throw std::runtime_error(strprintf("File version (%d) too high to be read.", nVersionRequired));
```

if `up-version` is too vague?

💬 maflcko commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416454459)
> > Using RPC requires an authenticated connection.
>
> Could you please elaborate on why RPC or authentication is not a feasible option for your use case?

I am not familiar with electrum protocol, but the issue description says:

> An indexer like the ones for the electrum protocol should work without having access to the RPC interface

💬 maflcko commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2416456307)
I don't think the REST interface will be removed any time soon, or ever, so if users want this, it seems fine to add. But no strong opinion.

🚀 fanquake merged a pull request: "build: Bump minimum supported macOS to 13.0"
(https://github.com/bitcoin/bitcoin/pull/31048)

🤔 mzumsande reviewed a pull request: "Halt processing of unrequested transactions v2"
(https://github.com/bitcoin/bitcoin/pull/30572#pullrequestreview-2372049564)
> See the latest state of the code (`23551ef`), which I think have been pushed before your comment publication. There is a `ExpectedTx()` method checking in the `TxRequestTracker` if the transaction has been requested either by wtxid or txid, orphan or not. Currently, Bitcoin Core is doing parent orphan fetching by txid: https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L4682
> so there should no parent orphan received as an unsolicited transaction, and not found as `REQUEST
...

💬 nvk commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2416464868)
Would love to see this happen.

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1802879192)
clang-format for new code :sweat_smile:

📝 Sjors reopened a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)
When a contributor maintains a fork of the repo, any pull request they make to their own fork, or to the main repository, will trigger two CI runs one for the branch push and one for the pull request.

This was fixed for Cirrus in e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33, but it relies on setting a custom variable which Github CI lacks.

This PR therefore disables Github CI runs on branch pushes for forks entirely.

After this PR, pushes made to git branches inside fork repositories will n
...

💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2416470424)
Oh wow that is remarkably well hidden! In that case I will change this PR to apply the same behaviour as Cirruss.

<img width="811" alt="SKIP_BRANCH_PUSH=true" src="https://github.com/user-attachments/assets/16995ba5-e37d-4819-8a0b-ef83f830422b">

📝 Sjors converted_to_draft a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)
When a contributor maintains a fork of the repo, any pull request they make to their own fork, or to the main repository, will trigger two CI runs one for the branch push and one for the pull request.

This was fixed for Cirrus in e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33, but it relies on setting a custom variable which Github CI lacks.

This PR therefore disables Github CI runs on branch pushes for forks entirely.

After this PR, pushes made to git branches inside fork repositories will n
...

💬 stickies-v commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416494027)
The "should work without having access to the RPC interface" is the part that I'd like to understand better. For example, the electrs docs specifically mention the bitcoind jsonrpc authentication: https://github.com/romanz/electrs/blob/master/doc/config.md#electrs-configuration

💬 maflcko commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2416498223)
A bit surprising that all tests passed. Maybe a test-case can be modified, or a new test can be added, so that the test changes fail on master and pass on this pull request? Otherwise, this may be broken again in the future. Also, it is harder to review, because each reviewer will have to write the test themselves.

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1802901394)
Silent merge conflict with #30937

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802902566)
`HaveKeys` uses `std::vector<valtype>`

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802902988)
Done

💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802903174)
Updated the comment.