Bitcoin Core Github
43 subscribers
123K links
Download Telegram
manlovepang closed a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
📝 manlovepang reopened a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** 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 motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 i-am-yuvi commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591646519)
Concept ACK

Good catch @theStack!
💬 ryanofsky commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591648579)
> After the notification fires, the miner calling back in to request a CreateNewBlock run will now have to wait behind `cs_main` for random P2P stuff

I don't see why this would be true. There's no reason Mining.createNewBlock or BlockTemplate.waitNext methods need to use cs_main. cs_main is an implementation detail and not part of the interface. For example we could have:

```c++
std::shared_ptr<CBlockTemplate> g_block_template;
std::mutex g_block_template_mutex;
std::condition_variable g_block
...
🤔 mzumsande reviewed a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551617694)
Concept ACK

Good find! I think that `p2p_orphan_handling.py` has a similar [spot](https://github.com/bitcoin/bitcoin/blob/e7c479495509c068215b73f6df070af2d406ae15/test/functional/p2p_orphan_handling.py#L431).
💬 i-am-yuvi commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591741584)
It could be good followup! Can you also change that as well @theStack
👍 i-am-yuvi approved a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551649291)
Tested ACK e0b333682222927d64217b07bb8cfd7ff3139660
fanquake closed a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
📝 fanquake locked a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** 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 motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 maflcko commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591895415)
> It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.

Would it not be possible to use mocktime and sync_with_ping?
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1916121562)
Yeah, that should work. I'll fuzz with this for some time.
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2591928909)
`bbfc58a0af...69e076664d`: drop the tests changes from this PR because they were already merged via https://github.com/bitcoin/bitcoin/pull/31646. So here remains a CI change to detect future regressions.

If people want to run this manually, outside of the CI in a way that causes false positive (discussion above), what about something like this:

```diff
--- i/ci/test/03_test_script.sh
+++ w/ci/test/03_test_script.sh
@@ -184,13 +184,15 @@ function traffic_monitor_end()
# "permissio
...
💬 Sjors commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591934116)
> Having the Mining interface should only make it easier to implement support for other protocols because they can use it without needing to interact with validation code.

Which can be seen in action in RPC code that calls these methods.

>> An interface that pushes the new work itself, rather than a notification, would not have this issue.

> `waitNext` is providing an interface that pushes the new work itself. A reference to the new block is in the return value.

@TheBlueMatt when you opened
...
💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916158555)
In testnet3 and testnet4 the difficulty depends on the timestamp. After 20 minutes it drops to 1. These methods derive difficulty using the current timestamp.
💬 stratospher commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1914320847)
528b8cc: this would be more accurate for descendants.
```suggestion
} else if (pindexFirstInvalid != pindex) {
```
💬 stratospher commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1914366243)
7f88af8b: when `invalidateblock` is called multiple times, we would need to clear the `BLOCK_FAILED_VALID` from previous iteration so that both `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` don't happen together on the same block. ex: [this situation](https://github.com/bitcoin/bitcoin/blob/35bf426e02210c1bbb04926f4ca2e0285fbfcd11/test/functional/rpc_invalidateblock.py#L91)
```
if (it->second->nStatus & BLOCK_FAILED_VALID) {
it->second->nStatus = (it->second->nStatus ^ BLOCK_FAILED_VALID)
...
💬 maflcko commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916173716)
I'd say that test-only docs shouldn't be put in the real RPC docs for mainnet. If someone is running on t3/t4 they should be aware of the rules already. And if they are not aware, "if found now" doesn't help to explain the rules to them.
⚠️ fxcoudert opened an issue: "The 28.1 release is not tagged in github"
(https://github.com/bitcoin/bitcoin/issues/31660)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

In GitHub releases, 28.0 is still marked as latest. We are holding from updating in Homebrew until this is resolved: https://github.com/Homebrew/homebrew-core/pull/204066

### Expected behaviour

28.1 should be marked as latest release, to confirm it is authentic.

### Steps to reproduce

1. Go to https://github.com/bitcoin/bitcoin
2. Observe that latest release is 28.0

### Relevant log o
...
💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916184067)
Ok, I dropped "if found now".
💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2592035361)
Updated 922aa7a25650bdfd8428198f171f88915af1ffa0 -> 430d5feee9b8be3a85e85696c449753783301c2b ([kernel_cache_sizes_15](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_15) -> [kernel_cache_sizes_16](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_16), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_15..kernel_cache_sizes_16))

* Addressed @ryanofsky's [comment_1](https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1915387311), [c
...