📝 jackripper235 opened a pull request: "Delete .github directory"
(https://github.com/bitcoin/bitcoin/pull/28836)
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/28836)
<!--
*** 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
...
💬 jackripper235 commented on pull request "Delete .github directory":
(https://github.com/bitcoin/bitcoin/pull/28836#issuecomment-1804911783)
@dependabot merge
(https://github.com/bitcoin/bitcoin/pull/28836#issuecomment-1804911783)
@dependabot merge
💬 jackripper235 commented on pull request "Delete .github directory":
(https://github.com/bitcoin/bitcoin/pull/28836#issuecomment-1804911863)
@jackripper235
(https://github.com/bitcoin/bitcoin/pull/28836#issuecomment-1804911863)
@jackripper235
📝 jackripper235 opened a pull request: "Delete .cirrus.yml"
(https://github.com/bitcoin/bitcoin/pull/28837)
@jackripper235
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`).
...
(https://github.com/bitcoin/bitcoin/pull/28837)
@jackripper235
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`).
...
💬 jackripper235 commented on pull request "Delete .cirrus.yml":
(https://github.com/bitcoin/bitcoin/pull/28837#issuecomment-1804914880)
.github/workflows
(https://github.com/bitcoin/bitcoin/pull/28837#issuecomment-1804914880)
.github/workflows
📝 jackripper235 opened a pull request: "Update CONTRIBUTING.md"
(https://github.com/bitcoin-core/gui/pull/776)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the c
...
(https://github.com/bitcoin-core/gui/pull/776)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the c
...
📝 Sjors opened a pull request: "test: add assumeutxo wallet test"
(https://github.com/bitcoin/bitcoin/pull/28838)
Extracted from #28616, this adds a (very) basic wallet test for assume utxo. It checks some circumstances where a backup can and can't be loaded.
(https://github.com/bitcoin/bitcoin/pull/28838)
Extracted from #28616, this adds a (very) basic wallet test for assume utxo. It checks some circumstances where a backup can and can't be loaded.
💬 Sjors commented on pull request "test: Check error details with assert_debug_log on the assumeutxo invalid hash dump - follow-up #28698":
(https://github.com/bitcoin/bitcoin/pull/28835#issuecomment-1804938605)
ACK 7de76853728b423339d17f39224cf20305da1832
(https://github.com/bitcoin/bitcoin/pull/28835#issuecomment-1804938605)
ACK 7de76853728b423339d17f39224cf20305da1832
💬 Sjors commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)
As discussed offline, the `utxo_snapshot.sh` script should have a sanity check to make the node isn't pruned below the snapshot height. On a pruned node you can only make a snapshot for a block that hasn't been pruned yet.
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)
As discussed offline, the `utxo_snapshot.sh` script should have a sanity check to make the node isn't pruned below the snapshot height. On a pruned node you can only make a snapshot for a block that hasn't been pruned yet.
✅ achow101 closed a pull request: "Delete .github directory"
(https://github.com/bitcoin/bitcoin/pull/28836)
(https://github.com/bitcoin/bitcoin/pull/28836)
📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/28836)
<!--
*** 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
...
(https://github.com/bitcoin/bitcoin/pull/28836)
<!--
*** 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
...
✅ achow101 closed a pull request: "Delete .cirrus.yml"
(https://github.com/bitcoin/bitcoin/pull/28837)
(https://github.com/bitcoin/bitcoin/pull/28837)
📝 achow101 locked a pull request: "Delete .cirrus.yml"
(https://github.com/bitcoin/bitcoin/pull/28837)
@jackripper235
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`).
...
(https://github.com/bitcoin/bitcoin/pull/28837)
@jackripper235
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`).
...
✅ achow101 closed a pull request: "Update CONTRIBUTING.md"
(https://github.com/bitcoin-core/gui/pull/776)
(https://github.com/bitcoin-core/gui/pull/776)
📝 achow101 locked a pull request: "Update CONTRIBUTING.md"
(https://github.com/bitcoin-core/gui/pull/776)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the c
...
(https://github.com/bitcoin-core/gui/pull/776)
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 improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the c
...
🤔 stratospher reviewed a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1724008322)
> These tests have in common that they use self.restart_node() with changing some unrelated extra_args, which would then cause --v2transport to be dropped.
thanks for the detailed explanation! so the problem was test failures happening when unrelated `extra_args` were passed during `TestNode` restart!
i was also initially confused about whether we really need an extra v2transport argument in `TestNode::__init()__` but https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1802573479 m
...
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1724008322)
> These tests have in common that they use self.restart_node() with changing some unrelated extra_args, which would then cause --v2transport to be dropped.
thanks for the detailed explanation! so the problem was test failures happening when unrelated `extra_args` were passed during `TestNode` restart!
i was also initially confused about whether we really need an extra v2transport argument in `TestNode::__init()__` but https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1802573479 m
...
💬 stratospher commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1388909333)
68a9001: there's a behaviour difference when test is run with `--v2transport` option and `TestNode`(initially `"v2transport=0"`) is restarted with:
- `extra_args` supplied => `TestNode` restarts as v2 node
- `extra_args` not supplied => `TestNode` restarts as v1 node
maybe when `extra_args` is `None`, we could remove `"v2transport"` stuff it gets from the old `self.extra_args` using something like `extra_args = list(filter(lambda s: not s.startswith("-v2transport="), extra_args))`?
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1388909333)
68a9001: there's a behaviour difference when test is run with `--v2transport` option and `TestNode`(initially `"v2transport=0"`) is restarted with:
- `extra_args` supplied => `TestNode` restarts as v2 node
- `extra_args` not supplied => `TestNode` restarts as v1 node
maybe when `extra_args` is `None`, we could remove `"v2transport"` stuff it gets from the old `self.extra_args` using something like `extra_args = list(filter(lambda s: not s.startswith("-v2transport="), extra_args))`?
👋 TheCharlatan's pull request is ready for review: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830)
(https://github.com/bitcoin/bitcoin/pull/28830)
🤔 naumenkogs reviewed a pull request: "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1724197196)
Not sure if you missed the two comments:
[one](https://github.com/bitcoin/bitcoin/pull/27600/commits/0c0f2a2c3664d5ea81ee31230ddca40863dd76dd#r1387600158)
[two](https://github.com/bitcoin/bitcoin/pull/27600/commits/0c0f2a2c3664d5ea81ee31230ddca40863dd76dd#r1387598983)
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1724197196)
Not sure if you missed the two comments:
[one](https://github.com/bitcoin/bitcoin/pull/27600/commits/0c0f2a2c3664d5ea81ee31230ddca40863dd76dd#r1387600158)
[two](https://github.com/bitcoin/bitcoin/pull/27600/commits/0c0f2a2c3664d5ea81ee31230ddca40863dd76dd#r1387598983)
💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1389032216)
Apparently, in the previous commit 75868022a904c1f77871abf962bf9b88a9c5faf6 you assign `forced = true;` even for non-`forceInbound` peers.
So basically if a node had 8 connections which are non-`forceInbound` but still did eviction, a real `forceInbound` connection won't be able to join (see how you count nForced). Is that intended? Seems like a bug to me.
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1389032216)
Apparently, in the previous commit 75868022a904c1f77871abf962bf9b88a9c5faf6 you assign `forced = true;` even for non-`forceInbound` peers.
So basically if a node had 8 connections which are non-`forceInbound` but still did eviction, a real `forceInbound` connection won't be able to join (see how you count nForced). Is that intended? Seems like a bug to me.