Bitcoin Core Github
44 subscribers
122K links
Download Telegram
maflcko closed an issue: "Incorrect amount in transaction page"
(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?
💬 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?
maflcko closed a pull request: "test: Extend stale_tip_peer_management test"
(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)
💬 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
🚀 glozow merged a pull request: "test: make v2transport arg in addconnection mandatory and few cleanups"
(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
💬 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.
💬 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
💬 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`.
💬 epiccurious commented on pull request "fuzz: remove unused `args` and `context` from `FuzzedWallet`":
(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
...
💬 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
...
🤔 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!
💬 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);
```
💬 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.
💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479649957)
in "node: BroadcastTransaction should also check tx fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/30bf6674b117cb320984ac66133a496f19166160):

Don't we have a `DEFAULT_TRANSACTION_MAXFEERATE`? Wouldn't it be better to use that here instead of 0?
💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479650353)
in "node: BroadcastTransaction should also check tx fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/30bf6674b117cb320984ac66133a496f19166160):

Same comment as above regarding defaults
💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479651121)
in "node: BroadcastTransaction should also check tx fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/30bf6674b117cb320984ac66133a496f19166160):

Same comment regarding defaults vs 0