Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1297022112)
> Should also abort on a force push, to avoid wasting CPU, no?

Done in #28282.
🤔 MarcoFalke reviewed a pull request: "ci: Ensure that only a single workflow processes `github.ref` at a time"
(https://github.com/bitcoin/bitcoin/pull/28282#pullrequestreview-1582331060)
ACK
💬 MarcoFalke commented on pull request "ci: Ensure that only a single workflow processes `github.ref` at a time":
(https://github.com/bitcoin/bitcoin/pull/28282#discussion_r1297036891)
```suggestion
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
```

why is this needed?
💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1682048469)
> > Maybe someone else without write access can confirm whether they can re-run on Cirrus?
>
> I can't re-run GH actions tasks but am able to re-run cirrus jobs.

Does closing a PR and following re-opening it work?
dergoegge closed a pull request: "util: Type-safe transaction identifiers"
(https://github.com/bitcoin/bitcoin/pull/28107)
📝 dergoegge reopened a pull request: "util: Type-safe transaction identifiers"
(https://github.com/bitcoin/bitcoin/pull/28107)
We currently have two different identifiers for transactions: `txid` (refering to the hash of a transaction without witness data) and `wtxid` (referring to the hash of a transaction including witness data). Both are typed as `uint256` which could lead to type-safety bugs in which one transaction identifier type is passed where the other would be expected.

This PR introduces explicit `Txid` and `Wtxid` types that (if used) would cause compilation errors for such type confusion bugs.

(Only t
...
💬 dergoegge commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1682051697)
> Does closing a PR and following re-opening it work?

No, that just cancels all jobs and the gha one just continues running
🤔 ajtowns reviewed a pull request: "rpc: Add Arg() default helper"
(https://github.com/bitcoin/bitcoin/pull/28230#pullrequestreview-1582339621)
utACK fa0cbfc445fbbad4009833ad9f13fb56886111a5
💬 ajtowns commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297043205)
"checks" not "checkes"
💬 ajtowns commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297042546)
With `MaybeArg` always working, could make this:

```c++
static const UniValue* DetailMaybeArg(const std::vector<RPCArg>& params, const JSONRPCRequest* req, size_t i, bool null_ok)
{
...
if (!null_ok) CheckRequiredOrDefault(param);
...
}

#define TMPL_INST_MAYBE(ret_type, get_arg, return_code, return_code_null) \
template <> \
ret_type RPCHelpMan::get_arg(size_t i) const \
{
...
💬 ajtowns commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297043961)
Add "should not be used for optional arguments that do not have a default error" or similar?
💬 hebasto commented on pull request "ci: Ensure that only a single workflow processes `github.ref` at a time":
(https://github.com/bitcoin/bitcoin/pull/28282#discussion_r1297049828)
Thanks! Updated.
💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682070095)
> Are you still working on this?

I wonder if this is still meaningful now that #27675 has landed
🤔 MarcoFalke reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1582115745)
lgtm, one doc nit.


Approach ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0, though I have not closely reviewed e4ffabbffacc4b890d393aafcc8286916ef887d8 🏁

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/Kl
...
💬 MarcoFalke commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1297021567)
a70beafdb22564043dc24fc98133fdadbaf77d8a: This seems fragile, because someone may assume the sequence number is both unique for any tx, and strictly monotonically increasing for any child tx. Both assumptions are violated here, and it could make sense to update the doxygen of `CTxMemPoolEntry.m_sequence` for that?


Also, may be better to drop the `entry_` prefix from the `CTxMemPoolEntry::entry_sequence` member, since the scope is already clear from the surrounding class. Also, could drop th
...
💬 mehran636311 commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596)
Pellse full item my account open and stat
💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682074827)
Maybe the test? Though, I haven't looked at it recently.
👍 theStack approved a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941#pullrequestreview-1582383927)
ACK fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7

Seems reasonable to me to ensure that the RPC call has indeed reached the node before continuing, and should prevent races as described in #26962.
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297075358)
I don't like macros, so I'll leave as-is for now, because my version has less lines of code inside a macro
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297087713)
thx, fixed