👍 hodlinator approved a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423920012)
ACK ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba
Implemented my suggestions (except comment removal suggestion) + broke out `parse_size()` since my last review.
*util_string_tests* tests passed locally.
Left one comment, but nothing blocking.
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423920012)
ACK ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba
Implemented my suggestions (except comment removal suggestion) + broke out `parse_size()` since my last review.
*util_string_tests* tests passed locally.
Left one comment, but nothing blocking.
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834470480)
sdfsdfsd
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834470480)
sdfsdfsd
💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950)
As you say, only the length and type are no longer checked. This PR now implements rudimentary checking of flags, width and precision.
```suggestion
// Length and type in "[flags][width][.precision][length]type"
// is not checked. Parsing continues with the next '%'.
```
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950)
As you say, only the length and type are no longer checked. This PR now implements rudimentary checking of flags, width and precision.
```suggestion
// Length and type in "[flags][width][.precision][length]type"
// is not checked. Parsing continues with the next '%'.
```
💬 darosior commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1834499475)
This is not necessary anymore.
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1834499475)
This is not necessary anymore.
📝 naiyoma opened a pull request: "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript"
(https://github.com/bitcoin/bitcoin/pull/31256)
Closes #27391
Add support for displaying the (decompiled) P2SH redeemScript and P2WSH
witnessScript when calling `getrawtransaction and getblock....2.`
I used this [guide](https://naiyoma.github.io/) to create transactions and below are outputs from p2sh/p2wsh , getrawtransaction and getblock 2 on regtest
```
bitcoin-cli -regtest getrawtransaction $TXID 1 | jq -r '.vin[].redeemScript.asm
2 0332df518c53eba135966d45b350e7a9bbf984
...
(https://github.com/bitcoin/bitcoin/pull/31256)
Closes #27391
Add support for displaying the (decompiled) P2SH redeemScript and P2WSH
witnessScript when calling `getrawtransaction and getblock....2.`
I used this [guide](https://naiyoma.github.io/) to create transactions and below are outputs from p2sh/p2wsh , getrawtransaction and getblock 2 on regtest
```
bitcoin-cli -regtest getrawtransaction $TXID 1 | jq -r '.vin[].redeemScript.asm
2 0332df518c53eba135966d45b350e7a9bbf984
...
💬 fanquake commented on pull request "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript":
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464920585)
I think this is already being worked on in #31252.
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464920585)
I think this is already being worked on in #31252.
✅ naiyoma closed a pull request: "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript"
(https://github.com/bitcoin/bitcoin/pull/31256)
(https://github.com/bitcoin/bitcoin/pull/31256)
💬 polespinasa commented on pull request "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript":
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464938959)
> I think this is already being worked on in #31252.
@naiyoma please feel free to contribute in my PR if you want to work on this. P2SH still on TODO, only P2WSH is implemented and solving functional test still also on TODO.
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464938959)
> I think this is already being worked on in #31252.
@naiyoma please feel free to contribute in my PR if you want to work on this. P2SH still on TODO, only P2WSH is implemented and solving functional test still also on TODO.
💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#issuecomment-2464948588)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31022#issuecomment-2464948588)
Concept ACK
👍 maflcko approved a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423906139)
left some nit ideas for more tests, but this is good either way.
review ACK ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba 🕯
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+kr
...
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423906139)
left some nit ideas for more tests, but this is good either way.
review ACK ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba 🕯
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+kr
...
💬 maflcko commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834499010)
Could add new failing cases here as well?
```
FailFmtWithError<2>("%2$*$d", err_0_pos);
FailFmtWithError<2>("%2$*0$d", err_0_pos);
FailFmtWithError<3>("%3$*2$.*$f", err_0_pos);
FailFmtWithError<3>("%3$*2$.*0$f", err_0_pos);
```
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834499010)
Could add new failing cases here as well?
```
FailFmtWithError<2>("%2$*$d", err_0_pos);
FailFmtWithError<2>("%2$*0$d", err_0_pos);
FailFmtWithError<3>("%3$*2$.*$f", err_0_pos);
FailFmtWithError<3>("%3$*2$.*0$f", err_0_pos);
```
💬 maflcko commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834505494)
Same (below):
```
FailFmtWithError<1>("%*c", err_num);
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834505494)
Same (below):
```
FailFmtWithError<1>("%*c", err_num);
💬 maflcko commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834504076)
Same:
```
FailFmtWithError<2>("%2$*d", err_mix);
FailFmtWithError<2>("%*2$d", err_mix);
FailFmtWithError<2>("%.*3$d", err_mix);
FailFmtWithError<2>("%2$.*d", err_mix);
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834504076)
Same:
```
FailFmtWithError<2>("%2$*d", err_mix);
FailFmtWithError<2>("%*2$d", err_mix);
FailFmtWithError<2>("%.*3$d", err_mix);
FailFmtWithError<2>("%2$.*d", err_mix);
💬 Sjors commented on pull request "init: warn, don't error, when '-upnp' is set":
(https://github.com/bitcoin/bitcoin/pull/31198#issuecomment-2464977943)
cc @ryanofsky who recently refactored a lot of the settings code.
(https://github.com/bitcoin/bitcoin/pull/31198#issuecomment-2464977943)
cc @ryanofsky who recently refactored a lot of the settings code.
💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1834551086)
I still don't know if the current docs are correct and useful.
Why would `ETIMEDOUT` only happen on warmup? Why is it even relevant to mention anything here?
I think it would be clearer to just mention that the three are treated equal to "-342 Service unavailable", which is already explained above?
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1834551086)
I still don't know if the current docs are correct and useful.
Why would `ETIMEDOUT` only happen on warmup? Why is it even relevant to mention anything here?
I think it would be clearer to just mention that the three are treated equal to "-342 Service unavailable", which is already explained above?
💬 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-2465021413)
> because the `BlockTemplate` object provides information about the last block sent to the client.
Good point, the lack of such a reference has been a source of headaches.
I plan to look into this next week.
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2465021413)
> because the `BlockTemplate` object provides information about the last block sent to the client.
Good point, the lack of such a reference has been a source of headaches.
I plan to look into this next week.
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2465040293)
@dergoegge I made a note to re-introduce a verification method for blocks. IIUC the use case would be to verify externally generated blocks, similar to how `proposal` works over RPC - but faster because it avoids RPC overhead.
I'd rather design that method from scratch (dropping the current implementation). E.g. it should provide useful feedback if our tip doesn't match, maybe even have an option to reorg in that case, etc. This is not needed for Stratum v2, so it's lower on my list of priori
...
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2465040293)
@dergoegge I made a note to re-introduce a verification method for blocks. IIUC the use case would be to verify externally generated blocks, similar to how `proposal` works over RPC - but faster because it avoids RPC overhead.
I'd rather design that method from scratch (dropping the current implementation). E.g. it should provide useful feedback if our tip doesn't match, maybe even have an option to reorg in that case, etc. This is not needed for Stratum v2, so it's lower on my list of priori
...
💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2465046092)
Marking draft because I might drop this method in favor of the approach suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2450648747
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2465046092)
Marking draft because I might drop this method in favor of the approach suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2450648747
📝 Sjors converted_to_draft a pull request: "Add waitFeesChanged() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls `getTransactionsUpdated` once per second. This returns `true` anytime a transacti
...
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls `getTransactionsUpdated` once per second. This returns `true` anytime a transacti
...
💬 Sjors commented on pull request "refactor: mining interface 30955 followups":
(https://github.com/bitcoin/bitcoin/pull/31197#discussion_r1834587201)
Will do if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31197#discussion_r1834587201)
Will do if I need to retouch.