💬 Sjors commented on pull request "Switch hardened derivation marker to h":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1187496219)
Added (plus "and similar")
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1187496219)
Added (plus "and similar")
🤔 darosior reviewed a pull request: "Switch hardened derivation marker to h"
(https://github.com/bitcoin/bitcoin/pull/26076#pullrequestreview-1416887139)
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
(https://github.com/bitcoin/bitcoin/pull/26076#pullrequestreview-1416887139)
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-1538431662)
I'll rebase soon(tm).
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-1538431662)
I'll rebase soon(tm).
🚀 fanquake merged a pull request: "msvc: Cleanup after upgrading libsecp256k1 up to 0.3.0"
(https://github.com/bitcoin/bitcoin/pull/27580)
(https://github.com/bitcoin/bitcoin/pull/27580)
💬 pinheadmz commented on pull request "msvc: Cleanup after upgrading libsecp256k1 up to 0.3.0":
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538433259)
ACK
I noticed when switching locally between old and new branches that some untracked files got left over:
```
--> git status
On branch master
Your branch is up to date with 'origin/master'.
Untracked files:
(use "git add <file>..." to include in what will be committed)
src/secp256k1/src/libsecp256k1-config.h
src/secp256k1/src/libsecp256k1-config.h.in
src/secp256k1/src/stamp-h1
nothing added to commit but untracked files present (use "git add" to track)
```
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538433259)
ACK
I noticed when switching locally between old and new branches that some untracked files got left over:
```
--> git status
On branch master
Your branch is up to date with 'origin/master'.
Untracked files:
(use "git add <file>..." to include in what will be committed)
src/secp256k1/src/libsecp256k1-config.h
src/secp256k1/src/libsecp256k1-config.h.in
src/secp256k1/src/stamp-h1
nothing added to commit but untracked files present (use "git add" to track)
```
💬 pinheadmz commented on pull request "msvc: Cleanup after upgrading libsecp256k1 up to 0.3.0":
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538447056)
Actually I still get the untracked files warning after merge -- am i doing something wrong? Or would it make any sense to add these to gitignore?
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538447056)
Actually I still get the untracked files warning after merge -- am i doing something wrong? Or would it make any sense to add these to gitignore?
💬 Sjors commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#issuecomment-1538451521)
Concept ACK on clarifying and returning both variants.
It would useful to peruse a few other projects to see how they use `getmempoolentry` and `getrawmempool`, to determine if this is a breaking change.
(https://github.com/bitcoin/bitcoin/pull/27591#issuecomment-1538451521)
Concept ACK on clarifying and returning both variants.
It would useful to peruse a few other projects to see how they use `getmempoolentry` and `getrawmempool`, to determine if this is a breaking change.
👍 ryanofsky approved a pull request: "refactor: Move chain constants to the util library"
(https://github.com/bitcoin/bitcoin/pull/27491#pullrequestreview-1416874927)
Code review ACK 95744f9cf1f143b6449a5046a914557b3886e3a2.
I think this looks good in it's current form, but the PR would have less churn and be simpler overall if the last commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2) was done earlier and became the second commit before commit "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)
(https://github.com/bitcoin/bitcoin/pull/27491#pullrequestreview-1416874927)
Code review ACK 95744f9cf1f143b6449a5046a914557b3886e3a2.
I think this looks good in it's current form, but the PR would have less churn and be simpler overall if the last commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2) was done earlier and became the second commit before commit "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)
💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187490948)
In commit "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)
Could this new method be added in an earlier commit? This commit is very long and all the other changes in the commit are mechanical changes except this one method, so the new code seems to get lost.
(This would also open the door to automating the other changes in this commit with a scripted-diff, though that would still be awkward not and not necessary IMO)
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187490948)
In commit "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)
Could this new method be added in an earlier commit? This commit is very long and all the other changes in the commit are mechanical changes except this one method, so the new code seems to get lost.
(This would also open the door to automating the other changes in this commit with a scripted-diff, though that would still be awkward not and not necessary IMO)
💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187504635)
In commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2)
Would be helpful to have a code comment describing what this does. Like "Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a recognized chain name was set, or as a string if an unrecognized chain name was set. Raise an exception if an invalid combination of flags was provided.
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187504635)
In commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2)
Would be helpful to have a code comment describing what this does. Like "Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a recognized chain name was set, or as a string if an unrecognized chain name was set. Raise an exception if an invalid combination of flags was provided.
💬 ryanofsky commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187514090)
In commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2)
It seems like this commit could be moved before the "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)" commit, which would simplify that commit, and avoid for the `ArgsManager::GetChainType()` method to change after it is added. I think the only change you would need to make to the code in this commit to move it earlier would be
...
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1187514090)
In commit "refactor: Avoid needless chain type string conversions" (95744f9cf1f143b6449a5046a914557b3886e3a2)
It seems like this commit could be moved before the "refactor: Replace string chain name constants with ChainTypes" (ca13faf0cc8acbaf425ac69c783415465403b5a3)" commit, which would simplify that commit, and avoid for the `ArgsManager::GetChainType()` method to change after it is added. I think the only change you would need to make to the code in this commit to move it earlier would be
...
💬 fanquake commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1538483661)
cc @achow101 @Xekyo @furszy
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1538483661)
cc @achow101 @Xekyo @furszy
⚠️ fanquake opened an issue: "ci: failure in feature_taproot.py (TSAN)"
(https://github.com/bitcoin/bitcoin/issues/27595)
master at 26cb32c02d76d6635c67942a5eeb70a6199df69d. https://cirrus-ci.com/task/5769539133112320?logs=ci#L3369:
```bash
node0 2023-05-08T14:19:47.988934Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=sendrawtransaction user=__cookie__
test 2023-05-08T14:19:47.989000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin
...
(https://github.com/bitcoin/bitcoin/issues/27595)
master at 26cb32c02d76d6635c67942a5eeb70a6199df69d. https://cirrus-ci.com/task/5769539133112320?logs=ci#L3369:
```bash
node0 2023-05-08T14:19:47.988934Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=sendrawtransaction user=__cookie__
test 2023-05-08T14:19:47.989000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin
...
💬 instagibbs commented on issue "Avoid serving stale fees":
(https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1538504504)
Here's my guess on how this happens and explaining the behavior I'm seeing
1) On node restart, there is no persistence of mempoolminfee
2) While mempool is importing, node receives transactions well below the old mempoolminfee, enters into mempool
3) mempool finishes loading(with some failures), resulting in no trimming on startup
4) `mempoolminfee` seems to be feeding into `estimatesmartfee` as a floor, so if node restarts, it starts naively giving out min incremental feerate
5) In `estima
...
(https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1538504504)
Here's my guess on how this happens and explaining the behavior I'm seeing
1) On node restart, there is no persistence of mempoolminfee
2) While mempool is importing, node receives transactions well below the old mempoolminfee, enters into mempool
3) mempool finishes loading(with some failures), resulting in no trimming on startup
4) `mempoolminfee` seems to be feeding into `estimatesmartfee` as a floor, so if node restarts, it starts naively giving out min incremental feerate
5) In `estima
...
💬 hebasto commented on pull request "msvc: Cleanup after upgrading libsecp256k1 up to 0.3.0":
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538509995)
@pinheadmz
> Actually I still get the untracked files warning after merge -- am i doing something wrong?
Try to clean your local repo first. For example, `git clean -xdff`.
(https://github.com/bitcoin/bitcoin/pull/27580#issuecomment-1538509995)
@pinheadmz
> Actually I still get the untracked files warning after merge -- am i doing something wrong?
Try to clean your local repo first. For example, `git clean -xdff`.
💬 fanquake commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538515538)
@TheCharlatan can you update this to drop 3e76aff9d4709f44d6439cd0cbc2fd6c90cae6ab, or I think we could just close this?
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538515538)
@TheCharlatan can you update this to drop 3e76aff9d4709f44d6439cd0cbc2fd6c90cae6ab, or I think we could just close this?
📝 jamesob opened a pull request: "assumeutxo (2)"
(https://github.com/bitcoin/bitcoin/pull/27596)
- Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
- Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
- Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.
---
This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to cha
...
(https://github.com/bitcoin/bitcoin/pull/27596)
- Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
- Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
- Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.
---
This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to cha
...
💬 jamesob commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1538525518)
Closing this as replaced by https://github.com/bitcoin/bitcoin/pull/27596.
> I get a bunch of these warnings, that I don't get on master:
Thanks for spotting this @Sjors; one-line removal fixed in the new PR.
(https://github.com/bitcoin/bitcoin/pull/15606#issuecomment-1538525518)
Closing this as replaced by https://github.com/bitcoin/bitcoin/pull/27596.
> I get a bunch of these warnings, that I don't get on master:
Thanks for spotting this @Sjors; one-line removal fixed in the new PR.
✅ jamesob closed a pull request: "assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/15606)
(https://github.com/bitcoin/bitcoin/pull/15606)
⚠️ brandonpille opened an issue: "rpc: Allow importing wallets by data instead of by filename"
(https://github.com/bitcoin/bitcoin/issues/27597)
### Please describe the feature you'd like to see added.
Right now we can only import wallets by filename. This means you can only execute that rpc if you are executing it from the system where the bitcoin node is running. But it could be that you are sending the rpc request from outside the node. In that case you don't have access to the file system. So it would be handy to allow passing the content of a wallet dump file instead and also allowing dumping the content not in a file but in the rp
...
(https://github.com/bitcoin/bitcoin/issues/27597)
### Please describe the feature you'd like to see added.
Right now we can only import wallets by filename. This means you can only execute that rpc if you are executing it from the system where the bitcoin node is running. But it could be that you are sending the rpc request from outside the node. In that case you don't have access to the file system. So it would be handy to allow passing the content of a wallet dump file instead and also allowing dumping the content not in a file but in the rp
...