✅ maflcko closed an issue: "Monkey Test Feature (MTF)"
(https://github.com/bitcoin/bitcoin/issues/29389)
(https://github.com/bitcoin/bitcoin/issues/29389)
💬 maflcko commented on issue "Monkey Test Feature (MTF)":
(https://github.com/bitcoin/bitcoin/issues/29389#issuecomment-1929174692)
I also don't understand your "test". If there is a bug in the wallet, calling the wallet code with a different amount is not going to fix the bug. If you are worried about the created transaction, my recommendation would be to inspect it before broadcasting, as mentioned above.
In any case, no one is holding anyone back from implementing a "test". Pull requests are welcome, but I am going to close the issue for now, unless there is something specific to discuss.
(https://github.com/bitcoin/bitcoin/issues/29389#issuecomment-1929174692)
I also don't understand your "test". If there is a bug in the wallet, calling the wallet code with a different amount is not going to fix the bug. If you are worried about the created transaction, my recommendation would be to inspect it before broadcasting, as mentioned above.
In any case, no one is holding anyone back from implementing a "test". Pull requests are welcome, but I am going to close the issue for now, unless there is something specific to discuss.
💬 maflcko commented on issue "When I make a highly concurrent request to bitcoin core, a new block appears and all my requests get blocked":
(https://github.com/bitcoin/bitcoin/issues/29384#issuecomment-1929183297)
Related: #29386?
(https://github.com/bitcoin/bitcoin/issues/29384#issuecomment-1929183297)
Related: #29386?
✅ maflcko closed an issue: "Incorrect amount in transaction page"
(https://github.com/bitcoin/bitcoin/issues/29378)
(https://github.com/bitcoin/bitcoin/issues/29378)
💬 maflcko commented on issue "Incorrect amount in transaction page":
(https://github.com/bitcoin/bitcoin/issues/29378#issuecomment-1929186223)
Closing for now, as it seems to be fixed?
(https://github.com/bitcoin/bitcoin/issues/29378#issuecomment-1929186223)
Closing for now, as it seems to be fixed?
💬 maflcko commented on issue "build warnings in outputtype.cpp: may be used uninitialized":
(https://github.com/bitcoin/bitcoin/issues/29359#issuecomment-1929188392)
So this may be an issue of Ubuntu/Debian refusing to apply fixes to their g++ for some reason, once released?
(https://github.com/bitcoin/bitcoin/issues/29359#issuecomment-1929188392)
So this may be an issue of Ubuntu/Debian refusing to apply fixes to their g++ for some reason, once released?
✅ maflcko closed a pull request: "test: Extend stale_tip_peer_management test"
(https://github.com/bitcoin/bitcoin/pull/23352)
(https://github.com/bitcoin/bitcoin/pull/23352)
🚀 glozow merged a pull request: "test: p2p: adhere to typical VERSION message protocol flow"
(https://github.com/bitcoin/bitcoin/pull/29353)
(https://github.com/bitcoin/bitcoin/pull/29353)
💬 glozow commented on pull request "test: make v2transport arg in addconnection mandatory and few cleanups":
(https://github.com/bitcoin/bitcoin/pull/29356#issuecomment-1929276821)
ACK e7fd70f4b6
(https://github.com/bitcoin/bitcoin/pull/29356#issuecomment-1929276821)
ACK e7fd70f4b6
🚀 glozow merged a pull request: "test: make v2transport arg in addconnection mandatory and few cleanups"
(https://github.com/bitcoin/bitcoin/pull/29356)
(https://github.com/bitcoin/bitcoin/pull/29356)
💬 sicelo7 commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1479590947)
Ye
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1479590947)
Ye
💬 Sjors commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1479616697)
My initial worry when seeing this PR was that it would touch mainnet consensus code in order to support these parameters. But clearly it doesn't, which is nice.
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1479616697)
My initial worry when seeing this PR was that it would touch mainnet consensus code in order to support these parameters. But clearly it doesn't, which is nice.
💬 Sjors commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929320537)
There was also the idea of dropping this number entirely: https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-410472259
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929320537)
There was also the idea of dropping this number entirely: https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-410472259
💬 maflcko commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929346759)
> Though see also https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-750715082
This is `nTx`, not `nChainTx`.
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929346759)
> Though see also https://github.com/bitcoin/bitcoin/pull/13875#issuecomment-750715082
This is `nTx`, not `nChainTx`.
💬 epiccurious commented on pull request "fuzz: remove unused `args` and `context` from `FuzzedWallet`":
(https://github.com/bitcoin/bitcoin/pull/29388#issuecomment-1929380707)
utACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac
(https://github.com/bitcoin/bitcoin/pull/29388#issuecomment-1929380707)
utACK b14298c5bca395e5ed6a27fe1758c0d1f4b824ac
💬 maflcko commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929388354)
For reference, I removed `nChainTx` from consensus in https://github.com/bitcoin/bitcoin/pull/29349. The next steps would be to add a member to `CChain` to account for the sum of `nTx`, and update it when `SetTip` is called. Then, update the RPC, wallet and estimate functions to derive the `nChainTx` they need from the `nChainTx` at the tip, walking the relevant portion of the chain.
This is possible, but I am not sure if it is worth it, because:
* It is more code overall.
* The RPC, wall
...
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1929388354)
For reference, I removed `nChainTx` from consensus in https://github.com/bitcoin/bitcoin/pull/29349. The next steps would be to add a member to `CChain` to account for the sum of `nTx`, and update it when `SetTip` is called. Then, update the RPC, wallet and estimate functions to derive the `nChainTx` they need from the `nChainTx` at the tip, walking the relevant portion of the chain.
This is possible, but I am not sure if it is worth it, because:
* It is more code overall.
* The RPC, wall
...
💬 epiccurious commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#discussion_r1479680894)
The previous is commit per git blame is [test: Speed up cache creation](https://github.com/bitcoin/bitcoin/commit/fa473303972b7dad600d949dc9b303d8136cb7e7#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R486) dated May 17, 2019 by @maflcko.
`-disablewallet` arg was added earlier in [qa: Premine to deterministic address with -disablewallet](https://github.com/bitcoin/bitcoin/commit/faa669cbcd1fc799517b523b0f850e01b11bf40a#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd
...
(https://github.com/bitcoin/bitcoin/pull/29387#discussion_r1479680894)
The previous is commit per git blame is [test: Speed up cache creation](https://github.com/bitcoin/bitcoin/commit/fa473303972b7dad600d949dc9b303d8136cb7e7#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76R486) dated May 17, 2019 by @maflcko.
`-disablewallet` arg was added earlier in [qa: Premine to deterministic address with -disablewallet](https://github.com/bitcoin/bitcoin/commit/faa669cbcd1fc799517b523b0f850e01b11bf40a#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd
...
🤔 josibake reviewed a pull request: "Wallet: Add `maxfeerate` wallet startup option"
(https://github.com/bitcoin/bitcoin/pull/29278#pullrequestreview-1864949072)
Looking good, and thanks for adding more tests!
(https://github.com/bitcoin/bitcoin/pull/29278#pullrequestreview-1864949072)
Looking good, and thanks for adding more tests!
💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479646534)
in "node: BroadcastTransaction should also check tx fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/30bf6674b117cb320984ac66133a496f19166160):
```suggestion
const TransactionError err = BroadcastTransaction(node, tx, err_string, /*max_tx_fee=*/0, max_raw_tx_fee_rate, /*relay=*/true, /*wait_callback=*/true);
```
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479646534)
in "node: BroadcastTransaction should also check tx fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/30bf6674b117cb320984ac66133a496f19166160):
```suggestion
const TransactionError err = BroadcastTransaction(node, tx, err_string, /*max_tx_fee=*/0, max_raw_tx_fee_rate, /*relay=*/true, /*wait_callback=*/true);
```
💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479660495)
in "wallet: add maxfeerate wallet startup option" (https://github.com/bitcoin/bitcoin/pull/29278/commits/4165dc41289fd337e279f3986dfc1a84345c900e):
Instead of touching this line twice, what do you think about reordering the commits so that this commit is first and the changes to `BroadcastTransaction` come after it? This way, you avoid needing to first set `/*max_tx_feerate=*/CFeeRate(0)` only to remove it in this commit.
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479660495)
in "wallet: add maxfeerate wallet startup option" (https://github.com/bitcoin/bitcoin/pull/29278/commits/4165dc41289fd337e279f3986dfc1a84345c900e):
Instead of touching this line twice, what do you think about reordering the commits so that this commit is first and the changes to `BroadcastTransaction` come after it? This way, you avoid needing to first set `/*max_tx_feerate=*/CFeeRate(0)` only to remove it in this commit.