⚠️ henark opened an issue: "research: Investigate feasibility of applying formal modeling to critical C++ consensus logic"
(https://github.com/bitcoin/bitcoin/issues/32854)
Problem: There is a significant assurance gap between the formally verified libsecp256k1 library and the unverified C++ code that implements the consensus rules which use it. Academic work has focused on modeling the protocol, not the C++ implementation. This gap is where subtle, critical bugs like CVE-2018-17144 can arise.
Proposal: Initiate a research track to explore the application of formal verification tools for C++ (e.g., Frama-C++, VeriFast, TIS-Analyzer) to a small but critical piece o
...
(https://github.com/bitcoin/bitcoin/issues/32854)
Problem: There is a significant assurance gap between the formally verified libsecp256k1 library and the unverified C++ code that implements the consensus rules which use it. Academic work has focused on modeling the protocol, not the C++ implementation. This gap is where subtle, critical bugs like CVE-2018-17144 can arise.
Proposal: Initiate a research track to explore the application of formal verification tools for C++ (e.g., Frama-C++, VeriFast, TIS-Analyzer) to a small but critical piece o
...
💬 achow101 commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#issuecomment-3025100533)
Removed the `include_watchonly=True`.
(https://github.com/bitcoin/bitcoin/pull/32618#issuecomment-3025100533)
Removed the `include_watchonly=True`.
✅ achow101 closed an issue: "refactor: Standardize logging levels and increase granularity in P2P and mempool modules"
(https://github.com/bitcoin/bitcoin/issues/32851)
(https://github.com/bitcoin/bitcoin/issues/32851)
💬 achow101 commented on issue "refactor: Standardize logging levels and increase granularity in P2P and mempool modules":
(https://github.com/bitcoin/bitcoin/issues/32851#issuecomment-3025113896)
Please do not use AI to spam open issues.
(https://github.com/bitcoin/bitcoin/issues/32851#issuecomment-3025113896)
Please do not use AI to spam open issues.
✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32854)
(https://github.com/bitcoin/bitcoin/issues/32854)
✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32853)
(https://github.com/bitcoin/bitcoin/issues/32853)
✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/32852)
(https://github.com/bitcoin/bitcoin/issues/32852)
🤔 w0xlt reviewed a pull request: "test: check P2SH sigop count for coinbase tx"
(https://github.com/bitcoin/bitcoin/pull/32850#pullrequestreview-2976606694)
ACK https://github.com/bitcoin/bitcoin/pull/32850/commits/d6aaffcb11adcf47480fcc5081af9dcb732decf3
(https://github.com/bitcoin/bitcoin/pull/32850#pullrequestreview-2976606694)
ACK https://github.com/bitcoin/bitcoin/pull/32850/commits/d6aaffcb11adcf47480fcc5081af9dcb732decf3
💬 achow101 commented on pull request "doc: clarify that the "-j N" goes after the "--build build" part":
(https://github.com/bitcoin/bitcoin/pull/32846#issuecomment-3025180099)
ACK 0e9f409db3b7b08aef75ce39765b018b69cc8e9d
(https://github.com/bitcoin/bitcoin/pull/32846#issuecomment-3025180099)
ACK 0e9f409db3b7b08aef75ce39765b018b69cc8e9d
🚀 achow101 merged a pull request: "doc: clarify that the "-j N" goes after the "--build build" part"
(https://github.com/bitcoin/bitcoin/pull/32846)
(https://github.com/bitcoin/bitcoin/pull/32846)
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178357373)
... it would make the code clearer... and we can handle the errors within that function instead... Thanks! Checking it...
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178357373)
... it would make the code clearer... and we can handle the errors within that function instead... Thanks! Checking it...
💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3025223286)
Rebased a67f3b07d4a1ea9289d4f7d5d3536fe2033b67b4 -> 0a9a1940782fad8dc3494c4aacee0a06c030eb5c ([`pr/ipc-stop.15`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.15) -> [`pr/ipc-stop.16`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.16), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.15-rebase..pr/ipc-stop.16)) after https://github.com/bitcoin-core/libmultiprocess/pull/186 merge. Marking ready for review again
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3025223286)
Rebased a67f3b07d4a1ea9289d4f7d5d3536fe2033b67b4 -> 0a9a1940782fad8dc3494c4aacee0a06c030eb5c ([`pr/ipc-stop.15`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.15) -> [`pr/ipc-stop.16`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.16), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.15-rebase..pr/ipc-stop.16)) after https://github.com/bitcoin-core/libmultiprocess/pull/186 merge. Marking ready for review again
🤔 ishaanam reviewed a pull request: "test: check P2SH sigop count for coinbase tx"
(https://github.com/bitcoin/bitcoin/pull/32850#pullrequestreview-2976751489)
ACK d6aaffcb11adcf47480fcc5081af9dcb732decf3
(https://github.com/bitcoin/bitcoin/pull/32850#pullrequestreview-2976751489)
ACK d6aaffcb11adcf47480fcc5081af9dcb732decf3
🤔 w0xlt reviewed a pull request: "wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members"
(https://github.com/bitcoin/bitcoin/pull/32763#pullrequestreview-2976772441)
ACK https://github.com/bitcoin/bitcoin/pull/32763/commits/9a8fd16e67755e336de683c0158d4fdd2b4ddfe8.
`value_map` and `order_form` (and therefore `WalletOrderForm` and `WalletValueMap`) lack clarity, making replacing with well-defined fields a much better approach.
(https://github.com/bitcoin/bitcoin/pull/32763#pullrequestreview-2976772441)
ACK https://github.com/bitcoin/bitcoin/pull/32763/commits/9a8fd16e67755e336de683c0158d4fdd2b4ddfe8.
`value_map` and `order_form` (and therefore `WalletOrderForm` and `WalletValueMap`) lack clarity, making replacing with well-defined fields a much better approach.
🤔 ismaelsadeeq reviewed a pull request: "refactor: CFeeRate encapsulates FeeFrac internally"
(https://github.com/bitcoin/bitcoin/pull/32750#pullrequestreview-2976133077)
The current iteration is looking better, I will do another round of review and testing tomorrow.
Just a few nits after a short glance
(https://github.com/bitcoin/bitcoin/pull/32750#pullrequestreview-2976133077)
The current iteration is looking better, I will do another round of review and testing tomorrow.
Just a few nits after a short glance
💬 ismaelsadeeq commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178028452)
nit: be less ambiguous?
```suggestion
const CAmount feerate_per_kvb = GetFeePerK();
```
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178028452)
nit: be less ambiguous?
```suggestion
const CAmount feerate_per_kvb = GetFeePerK();
```
💬 ismaelsadeeq commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178446483)
nit:
includes are alphabetical
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2178446483)
nit:
includes are alphabetical
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178453208)
Also, RPC `migratewallet` calls the same function so I could refactor that instance too.
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2178453208)
Also, RPC `migratewallet` calls the same function so I could refactor that instance too.
💬 achow101 commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3025413663)
> The approach I suggested of just backing up all wallets as `{prefix}_{timestamp}.legacy.bak` files in the top-level wallets directory seems like it should make the backup files easier to find, identify, and clean up.
That seems reasonable to me.
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3025413663)
> The approach I suggested of just backing up all wallets as `{prefix}_{timestamp}.legacy.bak` files in the top-level wallets directory seems like it should make the backup files easier to find, identify, and clean up.
That seems reasonable to me.
💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3025430544)
> Would there be interest in adding a benchmark for the Trim() operation here, or rather in a follow-up?
That will be nice, I don't mind adding it here. I plan to do another round of review of the added tests and diff after my last review so if added will review it as well.
I try running the bench on the linked commit. `MakeTxGraph` seems to takes two parameters, but the bench call provided three arguments.
I removed `NUM_ACCEPTABLE_ITERS` arg this is the result.
| ns/o
...
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3025430544)
> Would there be interest in adding a benchmark for the Trim() operation here, or rather in a follow-up?
That will be nice, I don't mind adding it here. I plan to do another round of review of the added tests and diff after my last review so if added will review it as well.
I try running the bench on the linked commit. `MakeTxGraph` seems to takes two parameters, but the bench call provided three arguments.
I removed `NUM_ACCEPTABLE_ITERS` arg this is the result.
| ns/o
...