💬 starkshade commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390343981)
## 💡 Improvement Needed
**Lines:** 75-80
The list of contributors should be alphabetized for better readability and consistency.
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390343981)
## 💡 Improvement Needed
**Lines:** 75-80
The list of contributors should be alphabetized for better readability and consistency.
💬 starkshade commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344059)
## 💡 Improvement Needed
**Line:** 167
The `GetTarget` function is now called with `blockindex` instead of `tip`. While this likely aligns with the intent to report the target difficulty of the specific `blockindex` being serialized, it's crucial to verify that this change does not inadvertently alter the expected output if `tip` was being used for a specific historical or comparative purpose. Ensure `GetTarget(blockindex, pow_limit)` accurately reflects the target difficulty for the `bloc
...
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344059)
## 💡 Improvement Needed
**Line:** 167
The `GetTarget` function is now called with `blockindex` instead of `tip`. While this likely aligns with the intent to report the target difficulty of the specific `blockindex` being serialized, it's crucial to verify that this change does not inadvertently alter the expected output if `tip` was being used for a specific historical or comparative purpose. Ensure `GetTarget(blockindex, pow_limit)` accurately reflects the target difficulty for the `bloc
...
💬 starkshade commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344151)
## 💡 Improvement Needed
**Line:** 1497
There appears to be a typo in the documentation string. 'final_scriptwitness' should likely be 'final_scriptSig' and 'final_scriptWitness' for consistency with common naming conventions and the preceding field.
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344151)
## 💡 Improvement Needed
**Line:** 1497
There appears to be a typo in the documentation string. 'final_scriptwitness' should likely be 'final_scriptSig' and 'final_scriptWitness' for consistency with common naming conventions and the preceding field.
💬 starkshade commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344300)
## 💡 Improvement Needed
**Lines:** 4035-4036
The added numerical values `1183846025` and `2713372123` should be verified for correctness and relevance within the context of the `mainnet_alt.json` test data. If these are part of a sequence or specific data points, ensure their accuracy and purpose are well-documented or self-evident.
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390344300)
## 💡 Improvement Needed
**Lines:** 4035-4036
The added numerical values `1183846025` and `2713372123` should be verified for correctness and relevance within the context of the `mainnet_alt.json` test data. If these are part of a sequence or specific data points, ensure their accuracy and purpose are well-documented or self-evident.
💬 starkshade commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390345078)
## ⚠️ Issue
**Type:** Maintainability
**Severity:** Low
**Line:** 63
The `mine` function's `block.nBits` assignment logic is conditional on `height < 2016`. This logic is also implicitly checked later in `run_test` when asserting `mining_info['next']['bits']` and `mining_info['next']['target']`. Duplicating this logic increases the risk of inconsistencies if the retargeting rules change.
**Fix:**
Abstract the retargeting logic into a dedicated helper function or constant to ensur
...
(https://github.com/bitcoin/bitcoin/pull/33474#discussion_r2390345078)
## ⚠️ Issue
**Type:** Maintainability
**Severity:** Low
**Line:** 63
The `mine` function's `block.nBits` assignment logic is conditional on `height < 2016`. This logic is also implicitly checked later in `run_test` when asserting `mining_info['next']['bits']` and `mining_info['next']['target']`. Duplicating this logic increases the risk of inconsistencies if the retargeting rules change.
**Fix:**
Abstract the retargeting logic into a dedicated helper function or constant to ensur
...
💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390346800)
@starkshade Stop spamming LLM generated slop that you don't understand yourself, or even have looked at yourself.
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390346800)
@starkshade Stop spamming LLM generated slop that you don't understand yourself, or even have looked at yourself.
🤔 starkshade reviewed a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3283265620)
This is a significant release candidate update for Bitcoin Core, primarily focusing on adjustments to transaction fee policies and related configurations. It includes changes to default minimum relay and block creation feerates, updates versioning information, and modifies documentation and tests to reflect these policy changes. The changes appear to be well-contained within the policy and testing domains.
**Key Changes:**
- Updated version to 28.3.0rc1.
- Lowered the default minimum block
...
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3283265620)
This is a significant release candidate update for Bitcoin Core, primarily focusing on adjustments to transaction fee policies and related configurations. It includes changes to default minimum relay and block creation feerates, updates versioning information, and modifies documentation and tests to reflect these policy changes. The changes appear to be well-contained within the policy and testing domains.
**Key Changes:**
- Updated version to 28.3.0rc1.
- Lowered the default minimum block
...
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343614)
## 💡 Improvement Needed
**Line:** 3
The version number `_CLIENT_VERSION_MINOR` is hardcoded. For better maintainability and to avoid manual updates across different configuration files, consider using a more dynamic versioning strategy or a dedicated version management tool.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343614)
## 💡 Improvement Needed
**Line:** 3
The version number `_CLIENT_VERSION_MINOR` is hardcoded. For better maintainability and to avoid manual updates across different configuration files, consider using a more dynamic versioning strategy or a dedicated version management tool.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343984)
## 💡 Improvement Needed
**Line:** 4
The version number `v28.3.0rc1` in the NAME section should be updated to match the version specified in the `.TH` directive (line 2). This ensures consistency throughout the manual page.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343984)
## 💡 Improvement Needed
**Line:** 4
The version number `v28.3.0rc1` in the NAME section should be updated to match the version specified in the `.TH` directive (line 2). This ensures consistency throughout the manual page.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343881)
## 💡 Improvement Needed
**Line:** 2
The version number `v28.3.0rc1` and the date `September 2025` appear to be inconsistent. If this is a release candidate, the date should reflect the development stage. If it's a stable release, the version number should be updated. This inconsistency can confuse users about the software's current status.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343881)
## 💡 Improvement Needed
**Line:** 2
The version number `v28.3.0rc1` and the date `September 2025` appear to be inconsistent. If this is a release candidate, the date should reflect the development stage. If it's a stable release, the version number should be updated. This inconsistency can confuse users about the software's current status.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344184)
## 💡 Improvement Needed
**Line:** 708
The default value for `-minrelaytxfee` has been decreased from `0.00001` to `0.000001`. This change in a critical fee parameter warrants a more detailed explanation in the documentation regarding the reasoning and potential impact on transaction relaying and mining. Consider adding a brief note about the implications of this reduced default.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344184)
## 💡 Improvement Needed
**Line:** 708
The default value for `-minrelaytxfee` has been decreased from `0.00001` to `0.000001`. This change in a critical fee parameter warrants a more detailed explanation in the documentation regarding the reasoning and potential impact on transaction relaying and mining. Consider adding a brief note about the implications of this reduced default.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344408)
## 💡 Improvement Needed
**Line:** 4
The version number in the NAME section should be updated to `v28.3.0rc1` to accurately reflect the current release of `bitcoin-tx`.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344408)
## 💡 Improvement Needed
**Line:** 4
The version number in the NAME section should be updated to `v28.3.0rc1` to accurately reflect the current release of `bitcoin-tx`.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344078)
## 💡 Improvement Needed
**Line:** 18
The version number `v28.3.0rc1` in the DESCRIPTION section should be updated to match the version specified in the `.TH` directive (line 2) and the NAME section (line 4). This ensures consistency throughout the manual page.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344078)
## 💡 Improvement Needed
**Line:** 18
The version number `v28.3.0rc1` in the DESCRIPTION section should be updated to match the version specified in the `.TH` directive (line 2) and the NAME section (line 4). This ensures consistency throughout the manual page.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390345094)
## 💡 Improvement Needed
**Line:** 4
The version number `v28.3.0rc1` in the NAME section should be consistent with the version specified in the `.TH` directive (line 2) and the DESCRIPTION section (line 6). This ensures accurate and unified version information throughout the manual page.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390345094)
## 💡 Improvement Needed
**Line:** 4
The version number `v28.3.0rc1` in the NAME section should be consistent with the version specified in the `.TH` directive (line 2) and the DESCRIPTION section (line 6). This ensures accurate and unified version information throughout the manual page.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344715)
## 💡 Improvement Needed
**Line:** 4
The version number in the NAME section of the manual page should be updated to reflect the current release. The current entry shows 'v28.3.0rc1', which indicates a release candidate. For a stable release, this should be updated to the final version number.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344715)
## 💡 Improvement Needed
**Line:** 4
The version number in the NAME section of the manual page should be updated to reflect the current release. The current entry shows 'v28.3.0rc1', which indicates a release candidate. For a stable release, this should be updated to the final version number.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344263)
## 💡 Improvement Needed
**Line:** 735
The default value for `-blockmintxfee` has been significantly reduced from `0.00001` to `0.00000001`. This change has substantial implications for which transactions are considered for block inclusion. The documentation should include a clear explanation of the rationale behind this drastic reduction and any anticipated effects on transaction propagation and block composition.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344263)
## 💡 Improvement Needed
**Line:** 735
The default value for `-blockmintxfee` has been significantly reduced from `0.00001` to `0.00000001`. This change has substantial implications for which transactions are considered for block inclusion. The documentation should include a clear explanation of the rationale behind this drastic reduction and any anticipated effects on transaction propagation and block composition.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344607)
## 💡 Improvement Needed
**Line:** 2
The version number in the manual page should be updated to reflect the current release. The current entry shows 'v28.3.0rc1', which indicates a release candidate. For a stable release, this should be updated to the final version number.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344607)
## 💡 Improvement Needed
**Line:** 2
The version number in the manual page should be updated to reflect the current release. The current entry shows 'v28.3.0rc1', which indicates a release candidate. For a stable release, this should be updated to the final version number.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344497)
## 💡 Improvement Needed
**Line:** 12
The version number in the DESCRIPTION section should be updated to `v28.3.0rc1` to accurately reflect the current release of `bitcoin-tx`.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390344497)
## 💡 Improvement Needed
**Line:** 12
The version number in the DESCRIPTION section should be updated to `v28.3.0rc1` to accurately reflect the current release of `bitcoin-tx`.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343799)
## 💡 Improvement Needed
**Line:** 5
The version number `_CLIENT_VERSION_RC` is hardcoded. For better maintainability and to avoid manual updates across different configuration files, consider using a more dynamic versioning strategy or a dedicated version management tool.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390343799)
## 💡 Improvement Needed
**Line:** 5
The version number `_CLIENT_VERSION_RC` is hardcoded. For better maintainability and to avoid manual updates across different configuration files, consider using a more dynamic versioning strategy or a dedicated version management tool.
💬 starkshade commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390345189)
## 💡 Improvement Needed
**Line:** 6
The version number `v28.3.0rc1` in the DESCRIPTION section should be consistent with the version specified in the `.TH` directive (line 2) and the NAME section (line 4). This ensures accurate and unified version information throughout the manual page.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2390345189)
## 💡 Improvement Needed
**Line:** 6
The version number `v28.3.0rc1` in the DESCRIPTION section should be consistent with the version specified in the `.TH` directive (line 2) and the NAME section (line 4). This ensures accurate and unified version information throughout the manual page.