💬 George43566 commented on pull request "Remove now-unnecessary poll, fcntl includes from net(base).cpp":
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1527049841)
I can handle it
On Thu, 27 Apr 2023, 4:00 pm Christopher Coverdale, <
***@***.***> wrote:
> tACK
> I also ran netbase.cpp through the iwyu CI and seems like there maybe a
> few more unused imports, but I haven't thoroughly tested removing the other
> ones
>
> netbase.cpp should remove these lines:
> - #include <fcntl.h> // lines 25-25
> - #include <poll.h> // lines 29-29
> - #include <chrono> // lines 18-18
> - #include <cstdint> // lines 19-19
> - #include <limits> // lines 21-21
>
> —
>
...
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1527049841)
I can handle it
On Thu, 27 Apr 2023, 4:00 pm Christopher Coverdale, <
***@***.***> wrote:
> tACK
> I also ran netbase.cpp through the iwyu CI and seems like there maybe a
> few more unused imports, but I haven't thoroughly tested removing the other
> ones
>
> netbase.cpp should remove these lines:
> - #include <fcntl.h> // lines 25-25
> - #include <poll.h> // lines 29-29
> - #include <chrono> // lines 18-18
> - #include <cstdint> // lines 19-19
> - #include <limits> // lines 21-21
>
> —
>
...
💬 Empact commented on pull request "Remove now-unnecessary poll, fcntl includes from net(base).cpp":
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1527141145)
I'll update this PR on Sunday, just busy with http://btcpp.dev at the moment.
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1527141145)
I'll update this PR on Sunday, just busy with http://btcpp.dev at the moment.
💬 MarcoFalke commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1527228543)
Thanks. I think it is best for you to open a follow-up, unless people want me to push the commit here.
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1527228543)
Thanks. I think it is best for you to open a follow-up, unless people want me to push the commit here.
💬 fanquake commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1527240485)
> I think it is best for you to open a follow-up,
Lets do that.
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1527240485)
> I think it is best for you to open a follow-up,
Lets do that.
🚀 fanquake merged a pull request: "Bump python minimum version to 3.8"
(https://github.com/bitcoin/bitcoin/pull/27483)
(https://github.com/bitcoin/bitcoin/pull/27483)
💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1180304523)
Pushed a much cleaner version of this: introducing a `MultiSigningProvider` to wrap around both the wallet and external providers. Still as a commit on top, let me know what you think @achow101.
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1180304523)
Pushed a much cleaner version of this: introducing a `MultiSigningProvider` to wrap around both the wallet and external providers. Still as a commit on top, let me know what you think @achow101.
📝 fjahr opened a pull request: "test: Remove modinv python util helper function"
(https://github.com/bitcoin/bitcoin/pull/27538)
Since #27483 was merged the `modinv()` body is just one line calling pythons own implementation of `pow()`. We can just remove the function as it doesn't seem to add any value. Additionally the comment in the function is now outdated and the test is only testing two ways of doing modular inverse but both using python's `pow()` function.
(https://github.com/bitcoin/bitcoin/pull/27538)
Since #27483 was merged the `modinv()` body is just one line calling pythons own implementation of `pow()`. We can just remove the function as it doesn't seem to add any value. Additionally the comment in the function is now outdated and the test is only testing two ways of doing modular inverse but both using python's `pow()` function.
💬 sipa commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527470660)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527470660)
Concept ACK
💬 MarcoFalke commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329594)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329594)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
💬 MarcoFalke commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329919)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180329919)
```
test/functional/test_framework/util.py:18:1: F401 'unittest' imported but unused
💬 MarcoFalke commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1527477037)
```
contrib/devtools/security-check.py:246: error: "object" has no attribute "header" [attr-defined]
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1527477037)
```
contrib/devtools/security-check.py:246: error: "object" has no attribute "header" [attr-defined]
💬 Riahiamirreza commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527485466)
Can I work on it? Isn't it much hard as the first time contribution?
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527485466)
Can I work on it? Isn't it much hard as the first time contribution?
💬 michaelfolkson commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527511935)
@Riahiamirreza: You don't need to ask but stating you are planning to work on this is helpful for others. If you look at the linked attempt in #8849 it looks doable as a first contribution but I don't know your skillset, level of understanding.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527511935)
@Riahiamirreza: You don't need to ask but stating you are planning to work on this is helpful for others. If you look at the linked attempt in #8849 it looks doable as a first contribution but I don't know your skillset, level of understanding.
💬 kevkevinpal commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527515641)
Concept ACK also did a grep for modinv in /test and looks like you got all them
(https://github.com/bitcoin/bitcoin/pull/27538#issuecomment-1527515641)
Concept ACK also did a grep for modinv in /test and looks like you got all them
💬 fjahr commented on pull request "test: Remove modinv python util helper function":
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180372914)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/27538#discussion_r1180372914)
Thanks, fixed.
💬 Riahiamirreza commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527530419)
@michaelfolkson Thanks for your reply. I think I have a good background in c++ and I'm familiar with the source code. But to start is I really need help.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527530419)
@michaelfolkson Thanks for your reply. I think I have a good background in c++ and I'm familiar with the source code. But to start is I really need help.
💬 schildbach commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1527540750)
The mempool message is used by all wallets based on bitcoinj to learn about pending transactions, in both full blocks mode and also in filtered mode. The estimated install base is 10M+.
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1527540750)
The mempool message is used by all wallets based on bitcoinj to learn about pending transactions, in both full blocks mode and also in filtered mode. The estimated install base is 10M+.
💬 michaelfolkson commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527543952)
@Riahiamirreza: Look at that attempt at changing rawtransaction.cpp and then ask questions on IRC #bitcoin-core-pr-reviews or Bitcoin StackExchange about the code in that file or the PR.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1527543952)
@Riahiamirreza: Look at that attempt at changing rawtransaction.cpp and then ask questions on IRC #bitcoin-core-pr-reviews or Bitcoin StackExchange about the code in that file or the PR.
💬 Riahiamirreza commented on pull request "Updated the installation instructions for macOS":
(https://github.com/bitcoin/bitcoin/pull/27525#discussion_r1180401267)
nit, using colon at the end of the line may looks better
> From here, change your working directory to "bitcoin" by running:
(https://github.com/bitcoin/bitcoin/pull/27525#discussion_r1180401267)
nit, using colon at the end of the line may looks better
> From here, change your working directory to "bitcoin" by running:
👍 theStack approved a pull request: "test: Remove modinv python util helper function"
(https://github.com/bitcoin/bitcoin/pull/27538#pullrequestreview-1405943871)
ACK dc14ba08e6e502f3e31d935bcd053a287c6610ca
(https://github.com/bitcoin/bitcoin/pull/27538#pullrequestreview-1405943871)
ACK dc14ba08e6e502f3e31d935bcd053a287c6610ca