π¬ murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459539171)
I donβt think this condition can be hit.
AFAICT, there is only be a single scenario in which `vfBest` can contain an input selection that is in excess of the `max_input_weight`: `ApproximateBestSubset` initializes `vfBest` with all of `applicable_groups` without checking the weight. If it then never finds a better input selection throughout the exploration, it would be possible that it returns an overweight `vfBest`. Since `lowest_larger` is only set when it adheres to the weight limit, we e
...
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459539171)
I donβt think this condition can be hit.
AFAICT, there is only be a single scenario in which `vfBest` can contain an input selection that is in excess of the `max_input_weight`: `ApproximateBestSubset` initializes `vfBest` with all of `applicable_groups` without checking the weight. If it then never finds a better input selection throughout the exploration, it would be possible that it returns an overweight `vfBest`. Since `lowest_larger` is only set when it adheres to the weight limit, we e
...
π¬ murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459526141)
I donβt think we can ever hit this `if` block since we only set `lowest_larger` when itβs weight is less than the max_input_weight.
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459526141)
I donβt think we can ever hit this `if` block since we only set `lowest_larger` when itβs weight is less than the max_input_weight.
π€ furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1833561066)
I implemented @mzumsande's proposed changes because they maintain the existing workflow and document the previously obscure behavior. The objective of this PR is to fix the post-IBD stalling scenario. It would be good to leave the discussion about the "safety buffer" number for a follow-up.
That being said, I agree with @vasild that the number is quite over-conservative for no apparent reason and could be increased.
> 144 blocks may still be very conservative / excessive, but it doesn't se
...
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1833561066)
I implemented @mzumsande's proposed changes because they maintain the existing workflow and document the previously obscure behavior. The objective of this PR is to fix the post-IBD stalling scenario. It would be good to leave the discussion about the "safety buffer" number for a follow-up.
That being said, I agree with @vasild that the number is quite over-conservative for no apparent reason and could be increased.
> 144 blocks may still be very conservative / excessive, but it doesn't se
...
π¬ instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665430)
makes sense, removed
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665430)
makes sense, removed
π¬ instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665529)
makes sense, removed
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459665529)
makes sense, removed
π¬ instagibbs commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#issuecomment-1901026758)
fixed up the fuzz test, which was attempting larger than `MAX_STANDARD_TX_WEIGHT` coins and not actually rejecting them as they should have previously, causing fuzz failure. Invariants in test should work now.
(https://github.com/bitcoin/bitcoin/pull/29264#issuecomment-1901026758)
fixed up the fuzz test, which was attempting larger than `MAX_STANDARD_TX_WEIGHT` coins and not actually rejecting them as they should have previously, causing fuzz failure. Invariants in test should work now.
π hebasto opened a pull request: "Avoid non-self-contained Windows header"
(https://github.com/bitcoin-core/gui/pull/789)
Using the `windows.h` header guarantees correctness regardless of the content of other headers.
For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio
Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager.
Related to https://github.com/hebasto/bitcoin/pull/77.
(https://github.com/bitcoin-core/gui/pull/789)
Using the `windows.h` header guarantees correctness regardless of the content of other headers.
For more details, please refer to https://stackoverflow.com/questions/4845198/fatal-error-no-target-architecture-in-visual-studio
Fixes the MSVC build when using the upcoming CMake-based build system and Qt packages installed via the vcpkg package manager.
Related to https://github.com/hebasto/bitcoin/pull/77.
π€ BrandonOdiwuor reviewed a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1833607389)
Concept ACK 358561e54ef84f64b216ca8a4271ad17d91b1d51
Increasing code coverage
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1833607389)
Concept ACK 358561e54ef84f64b216ca8a4271ad17d91b1d51
Increasing code coverage
π¬ hebasto commented on pull request "Avoid non-self-contained Windows header":
(https://github.com/bitcoin-core/gui/pull/789#issuecomment-1901046564)
Friendly ping @sipsorcery :)
(https://github.com/bitcoin-core/gui/pull/789#issuecomment-1901046564)
Friendly ping @sipsorcery :)
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709324)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709324)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709417)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709417)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709496)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709496)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709630)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709630)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709663)
looks good, done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709663)
looks good, done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709766)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709766)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709852)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709852)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709932)
done
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459709932)
done
π¬ instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459710174)
makes sense, moved
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1459710174)
makes sense, moved
π¬ murchandamus commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459729161)
It would be better to make `max_input_weight` a fuzzed value.
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1459729161)
It would be better to make `max_input_weight` a fuzzed value.
π sipa opened a pull request: "Choose earliest-activatable as tie breaker between equal-work chains"
(https://github.com/bitcoin/bitcoin/pull/29284)
### Current logic
Since #3370, our logic for deciding which chain is active has been:
1. Among chains for which all transactions have been received, and are not known to contain invalid blocks...
2. Pick the one with the highest accumulated work
3. If there are multiple such chaintips, pick the one for which the tip was received first (full block data, not just the header)
The reason for this last condition is protecting against block withholding. If an attacker manages to mine a block,
...
(https://github.com/bitcoin/bitcoin/pull/29284)
### Current logic
Since #3370, our logic for deciding which chain is active has been:
1. Among chains for which all transactions have been received, and are not known to contain invalid blocks...
2. Pick the one with the highest accumulated work
3. If there are multiple such chaintips, pick the one for which the tip was received first (full block data, not just the header)
The reason for this last condition is protecting against block withholding. If an attacker manages to mine a block,
...