Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ“ dergoegge opened a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
πŸ’¬ dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2457369883)
cc @maflcko couldn't reopen the other PR after force pushing
πŸ“ dergoegge converted_to_draft a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
πŸ’¬ instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1829485990)
will take language if I touch PR again
πŸ’¬ Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829528839)
I think this was already the case before this PR? It seems that the currently implementation in solver.cpp doesn't handle `OP_0` correctly, while the python codebase does?

https://github.com/bitcoin/bitcoin/blob/03cff2c1421e5db59963eba1a845ef5dd318c275/test/functional/test_framework/script.py#L89

https://github.com/bitcoin/bitcoin/blob/03cff2c1421e5db59963eba1a845ef5dd318c275/src/script/solver.cpp#L61

This goes to the purpose of this PR, handle them consistently across codebases and re
...
πŸ’¬ Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2457443510)
Thanks for looking at this @mzumsande !

> I didn’t see earlier versions of this PR, but It’s not clear to me what the goal of this PR is from the OP / Title. Should the title be updated, is this still supposed to fix a bug?

Modified the title, hopefully its more clear.

> As mentioned above, `DecodeOP_N` / `EncodeOP_N` currently don't need to handle `OP_1NEGATE` in any of the current use cases - so I think that there should be some explanation why they should be changed to support it.

...
πŸ’¬ Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829540702)
In f2f8796 I modified `IsSmallInteger()` to handle `OP_1NEGATE` _and_ `OP_0` correctly. Previously only the python codebase handled `OP_0` correctly.
πŸ’¬ mzumsande commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2457459639)
> What about having P2P-seeds, an alternative to DNS-seeds, which are serving the P2P protocol and are used as addrfetch peers and they return only high-quality addresses from the crawler?

Yes, I think that could work.
Also, I wouldn't say that this particular downside I mentioned would be a blocker - even if it would take on average 1 minute instead of 20 seconds to find the initial 10 peers, that wouldn't be the end of the world, since it's just a one-time event for an empty addrman.
πŸ’¬ darosior commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2457482792)
Concept NACK. This introduces dead code and i don't think the justification for this meets the bar for touching consensus code.
πŸ’¬ mzumsande commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829569059)
I don't think that "correct" is the right term here, it's more about what these functions are intended to be used for. In this case, the comment `Test for "small positive integer"` indicates that the function wasn't designed to deal with `OP_0`, so if just the name could just rename it to `IsSmallPositiveInteger` to not introduce a change in behavior.
πŸ’¬ dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2457529378)
Looks like using the matrix feature works for this
πŸ‘‹ dergoegge's pull request is ready for review: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
πŸ’¬ marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829592727)
The upper target is the `nBits` value from the genesis block (0x1D00FFFF) and the lower one is the `nBits` from some mainnet block at a certain height (0x17058EBE). Using `SetCompact` on those values gives those two uint256 values. The work returned from the lower target multiplied by the longest possible chain (1600) in the test should be lower than `MinimumChainWork`.

I get 0x11fe50e0ab9cbcf864c0500 for the theoretical max amount of work a headers chain from this test could have. `MinimumC
...
πŸ’¬ marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829593835)
Done.
πŸ’¬ fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2457548171)
I could remove another `sleep` import line after https://github.com/bitcoin/bitcoin/pull/30942 was merged which was what the linter complained about. Should be trivial to re-ACK: https://github.com/bitcoin/bitcoin/compare/352b8209aa5327f7d369e2acc4d87f9767389a6b..f04529fc40f2f7c8e4e63d892cdcbe6c91dfbf96
πŸ’¬ fanquake commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2457600736)
Guix build:
```bash
67b7e95e0f1d45debad8a8cac4c2d349e3e01efcf62e0b109345896f296758d9 guix-build-788c1324f3d8/output/aarch64-linux-gnu/SHA256SUMS.part
cd3c67eaa5f6fadf0a73cdb1aa03563d720fc7fdae1b59aa495a0eda9d2695af guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu-debug.tar.gz
5c1eb4ff827336a9d113497459981e213844b87e232c33fc00317ce20302f761 guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu.tar.gz
dd78923f8fb7410a
...
πŸ‘ fanquake approved a pull request: "depends, doc: List packages required to build `qt` package separately"
(https://github.com/bitcoin/bitcoin/pull/31192#pullrequestreview-2416094490)
ACK 4747f030956d6876ec7cef4e2500a8579bc82146
πŸš€ fanquake merged a pull request: "depends, doc: List packages required to build `qt` package separately"
(https://github.com/bitcoin/bitcoin/pull/31192)
πŸ’¬ fanquake commented on pull request "doc: Fix word order in developer-notes.md":
(https://github.com/bitcoin/bitcoin/pull/31220#issuecomment-2457604363)
ACK 44939e5de1b37ccd85ef31268219c5866bd3ee3f
πŸš€ fanquake merged a pull request: "doc: Fix word order in developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/31220)