👍 tdb3 approved a pull request: "wallet: notify when preset + automatic inputs exceed max weight"
(https://github.com/bitcoin/bitcoin/pull/30309#pullrequestreview-2133551145)
re ACK for 72b226882fe2348a9a66aee1d8d21b4e2d275e68
Re-ran tests locally (exercising induced failures and received failures as expected).
`test_weight_limits()` implementations are almost identical between `wallet_fundrawtransaction` and `wallet_spend`. While this feels a bit redundant, these are separate functional tests so an alternative (like combining) wouldn't really make sense to do.
(https://github.com/bitcoin/bitcoin/pull/30309#pullrequestreview-2133551145)
re ACK for 72b226882fe2348a9a66aee1d8d21b4e2d275e68
Re-ran tests locally (exercising induced failures and received failures as expected).
`test_weight_limits()` implementations are almost identical between `wallet_fundrawtransaction` and `wallet_spend`. While this feels a bit redundant, these are separate functional tests so an alternative (like combining) wouldn't really make sense to do.
💬 tdb3 commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1649519927)
Saw that the same expected failure occurs here when commenting out `return util::Error...` for 72b226882fe2348a9a66aee1d8d21b4e2d275e68.
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1649519927)
Saw that the same expected failure occurs here when commenting out `return util::Error...` for 72b226882fe2348a9a66aee1d8d21b4e2d275e68.
💬 tdb3 commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1649519554)
Re-ran this test (comment out `return util::Error...`) for 72b226882fe2348a9a66aee1d8d21b4e2d275e68. Failed as expected.
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1649519554)
Re-ran this test (comment out `return util::Error...`) for 72b226882fe2348a9a66aee1d8d21b4e2d275e68. Failed as expected.
💬 am-sq commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1649531050)
I made the change to reflect different path string depending on OS. Let me know if this is what you meant. I was also debating doing an alternative way where I have 2 example types, one MacOS and other Unix, so that web docs like https://developer.bitcoin.org/reference/rpc/loadwallet.html show both examples.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1649531050)
I made the change to reflect different path string depending on OS. Let me know if this is what you meant. I was also debating doing an alternative way where I have 2 example types, one MacOS and other Unix, so that web docs like https://developer.bitcoin.org/reference/rpc/loadwallet.html show both examples.
📝 romanz opened a pull request: "rest: don't copy block data when sending binary response"
(https://github.com/bitcoin/bitcoin/pull/30321)
Also, change `HTTPRequest::WriteReply` to accept `std::string_view`.
(https://github.com/bitcoin/bitcoin/pull/30321)
Also, change `HTTPRequest::WriteReply` to accept `std::string_view`.
💬 maflcko commented on pull request "rest: don't copy block data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2183964434)
lgtm ACK 07770609afddaa1c730b431c207015f34b3a5f5f
A benchmark would be nice (for fun), but this looks good either way.
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2183964434)
lgtm ACK 07770609afddaa1c730b431c207015f34b3a5f5f
A benchmark would be nice (for fun), but this looks good either way.
💬 laanwj commented on pull request "rest: don't copy block data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2184047556)
LGTM ACK 07770609afddaa1c730b431c207015f34b3a5f5f
Nice, simple optimization.
(https://github.com/bitcoin/bitcoin/pull/30321#issuecomment-2184047556)
LGTM ACK 07770609afddaa1c730b431c207015f34b3a5f5f
Nice, simple optimization.
💬 laanwj commented on pull request "rest: don't copy block data when sending binary response":
(https://github.com/bitcoin/bitcoin/pull/30321#discussion_r1649705033)
Not sure if a `Span<byte>` or string_view is best suited here. In principle we're dealing with binary data here, not necessarily a string.
(https://github.com/bitcoin/bitcoin/pull/30321#discussion_r1649705033)
Not sure if a `Span<byte>` or string_view is best suited here. In principle we're dealing with binary data here, not necessarily a string.
📝 ismaelsadeeq opened a pull request: "[test]: prevent `create_self_transfer` failure when target weight is below tx weight"
(https://github.com/bitcoin/bitcoin/pull/30322)
This is a simple test PR that does two things:
1. Prevents `create_self_transfer` failure when the `target_weight` is too low, i.e., below the tx weight.
2. Addresses some review comments from https://github.com/bitcoin/bitcoin/pull/30162, which are:
- Use `WITNESS_SCALE_FACTOR` instead of the magic number 4.
- Use `PADDING_OFFSET` instead of the magic number 3.
- Raise an exception if the output value is less than or equal to zero in `create_self_transfer`.
This prevents cre
...
(https://github.com/bitcoin/bitcoin/pull/30322)
This is a simple test PR that does two things:
1. Prevents `create_self_transfer` failure when the `target_weight` is too low, i.e., below the tx weight.
2. Addresses some review comments from https://github.com/bitcoin/bitcoin/pull/30162, which are:
- Use `WITNESS_SCALE_FACTOR` instead of the magic number 4.
- Use `PADDING_OFFSET` instead of the magic number 3.
- Raise an exception if the output value is less than or equal to zero in `create_self_transfer`.
This prevents cre
...
💬 ismaelsadeeq commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#issuecomment-2184052503)
> > Would be nice to have this work for create_self_transfer_multi as well
>
> I will pick this up and the outstanding review comments.
Opened #30322 that addresses most comments here and also prevent `create_self_transfer` failure when target weight is below tx weight, you can test this by passing a low `target_weight` to `create_self_transfer`.
(https://github.com/bitcoin/bitcoin/pull/30162#issuecomment-2184052503)
> > Would be nice to have this work for create_self_transfer_multi as well
>
> I will pick this up and the outstanding review comments.
Opened #30322 that addresses most comments here and also prevent `create_self_transfer` failure when target weight is below tx weight, you can test this by passing a low `target_weight` to `create_self_transfer`.
🤔 fjahr reviewed a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2133844775)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2133844775)
Concept ACK
💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1649733840)
nit: Not sure if users understand what "base block" is. Maybe make it "snapshot base block".
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1649733840)
nit: Not sure if users understand what "base block" is. Maybe make it "snapshot base block".
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739144)
done
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739144)
done
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739149)
Not sure why this is still needed when we return above if `!first_unpruned.pprev` but I've added it since it won't hurt.
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739149)
Not sure why this is still needed when we return above if `!first_unpruned.pprev` but I've added it since it won't hurt.
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739156)
Cool, thanks! Added the fix and the test.
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739156)
Cool, thanks! Added the fix and the test.
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739250)
I disagree here. I think this is the correct use of an optional when we may or may not have a value to return. And I think it's easier to understand because the code is more expressive. Handing a magical `-1` around is just not intuitive IMO.
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739250)
I disagree here. I think this is the correct use of an optional when we may or may not have a value to return. And I think it's easier to understand because the code is more expressive. Handing a magical `-1` around is just not intuitive IMO.
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739281)
done
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1649739281)
done
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#issuecomment-2184088934)
Addressed feedback from @stickies-v and @ryanofsky, thank you!
(https://github.com/bitcoin/bitcoin/pull/29668#issuecomment-2184088934)
Addressed feedback from @stickies-v and @ryanofsky, thank you!
👍 BrandonOdiwuor approved a pull request: "rest: don't copy block data when sending binary response"
(https://github.com/bitcoin/bitcoin/pull/30321#pullrequestreview-2133880309)
Code Review ACK 07770609afddaa1c730b431c207015f34b3a5f5f
(https://github.com/bitcoin/bitcoin/pull/30321#pullrequestreview-2133880309)
Code Review ACK 07770609afddaa1c730b431c207015f34b3a5f5f
📝 bombastictranz opened a pull request: "bombastictranz/bitcoin"
(https://github.com/bitcoin/bitcoin/pull/30323)
<!--
*** 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/30323)
<!--
*** 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
...