💬 fanquake commented on pull request "chainparams: Remove seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2312208366)
> I'm in the process of fixing the seeder. It should be back online by the end of the week, if that's acceptable
No worries. We can revert this once it's back up, and we can see it working correctly.
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2312208366)
> I'm in the process of fixing the seeder. It should be back online by the end of the week, if that's acceptable
No worries. We can revert this once it's back up, and we can see it working correctly.
✅ fanquake closed an issue: "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does"
(https://github.com/bitcoin/bitcoin/issues/29911)
(https://github.com/bitcoin/bitcoin/issues/29911)
🚀 fanquake merged a pull request: "chainparams: Remove seed.bitcoinstats.com"
(https://github.com/bitcoin/bitcoin/pull/30720)
(https://github.com/bitcoin/bitcoin/pull/30720)
💬 0xB10C commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312264308)
Thank you for reporting this.
While issues in `b-msghand` can be problematic and it's good to report them, I don't think this report contains any actionable details. The gbd backtrace only contains memory addresses and no further debug information.
Unless these you start to see more frequent segmentation faults, I don't think it's worth keeping this issue open.
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312264308)
Thank you for reporting this.
While issues in `b-msghand` can be problematic and it's good to report them, I don't think this report contains any actionable details. The gbd backtrace only contains memory addresses and no further debug information.
Unless these you start to see more frequent segmentation faults, I don't think it's worth keeping this issue open.
💬 mzumsande commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312312701)
Was there anything unusual in the debug log before the crash?
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312312701)
Was there anything unusual in the debug log before the crash?
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312318158)
> Also #28509.
Ah nice! I forgot about it. I guess it also underlines that the issue lies in starting the process.
Maybe GHA has some kind of "Anti-Virus" software that may block some processes?
> > > This could potentially be an issue between two different tests as well, if they shut down/start up quickly enough to still be within the TCP `TIME_WAIT` period.
> >
> >
> > I don't think different tests affect each other, because the test_runner assigns non-overlapping spans of ports
...
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312318158)
> Also #28509.
Ah nice! I forgot about it. I guess it also underlines that the issue lies in starting the process.
Maybe GHA has some kind of "Anti-Virus" software that may block some processes?
> > > This could potentially be an issue between two different tests as well, if they shut down/start up quickly enough to still be within the TCP `TIME_WAIT` period.
> >
> >
> > I don't think different tests affect each other, because the test_runner assigns non-overlapping spans of ports
...
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732688924)
done
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732688924)
done
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732689092)
done
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732689092)
done
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732691023)
Yes, we don't want a short circuit, though in practice `add_extra_compact_tx` should always be true at this point in the code. I've added a comment describing what the expected changes can be.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1732691023)
Yes, we don't want a short circuit, though in practice `add_extra_compact_tx` should always be true at this point in the code. I've added a comment describing what the expected changes can be.
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2312348384)
Thanks @dergoegge @theStack @instagibbs! Addressed all comments.
> Slightly off-topic, but: on some commits like https://github.com/bitcoin/bitcoin/commit/4e58d84da95d97d64f563b5c3e116769cd9ff89b --color-moved=dimmed-zebra didn't detect any lines as move-only for me (with 2-3 exceptions that seem completely arbitrary), any suggestions for advanced git options and/or recommendations for other fancy diff tools that can cope with that?
I see that happen too :( very annoying. Sometimes if I se
...
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2312348384)
Thanks @dergoegge @theStack @instagibbs! Addressed all comments.
> Slightly off-topic, but: on some commits like https://github.com/bitcoin/bitcoin/commit/4e58d84da95d97d64f563b5c3e116769cd9ff89b --color-moved=dimmed-zebra didn't detect any lines as move-only for me (with 2-3 exceptions that seem completely arbitrary), any suggestions for advanced git options and/or recommendations for other fancy diff tools that can cope with that?
I see that happen too :( very annoying. Sometimes if I se
...
👍 brunoerg approved a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2263102976)
crACK fab0e834b8cf906c286ebbeed86eca8d65854f75
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2263102976)
crACK fab0e834b8cf906c286ebbeed86eca8d65854f75
💬 hodlinator commented on pull request "refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}":
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312424733)
> For the future: you could have made the commit here an actual scripted diff too.
Good point, thanks, still getting the hang of scripted diffs. Makes sense in case new stuff using `uint256S` gets merged into master before this.
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312424733)
> For the future: you could have made the commit here an actual scripted diff too.
Good point, thanks, still getting the hang of scripted diffs. Makes sense in case new stuff using `uint256S` gets merged into master before this.
💬 maflcko commented on pull request "refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}":
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312441346)
> > For the future: you could have made the commit here an actual scripted diff too.
>
> Good point, thanks, still getting the hang of scripted diffs. Makes sense in case new stuff using `uint256S` gets merged into master before this.
A scripted-diff won't help here, because it is applied on top of the commit it is based on, which won't change when master is changed, unless you rebase on every push to master.
Moreover, I think scripted-diffs should only be used when it makes sense. As a
...
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312441346)
> > For the future: you could have made the commit here an actual scripted diff too.
>
> Good point, thanks, still getting the hang of scripted diffs. Makes sense in case new stuff using `uint256S` gets merged into master before this.
A scripted-diff won't help here, because it is applied on top of the commit it is based on, which won't change when master is changed, unless you rebase on every push to master.
Moreover, I think scripted-diffs should only be used when it makes sense. As a
...
📝 maflcko opened a pull request: " lint: Speed up and fix flake8 checks "
(https://github.com/bitcoin/bitcoin/pull/30723)
The checks have many issues:
* Some checks that could in theory hide bugs are not applied -> Fix them and apply them going forward
* Some checks are redundant Python 2 checks, or of low value -> Remove them
* The checks are slow -> Speed them up from ~10 seconds to about ~20 milliseconds
(https://github.com/bitcoin/bitcoin/pull/30723)
The checks have many issues:
* Some checks that could in theory hide bugs are not applied -> Fix them and apply them going forward
* Some checks are redundant Python 2 checks, or of low value -> Remove them
* The checks are slow -> Speed them up from ~10 seconds to about ~20 milliseconds
📝 theStack opened a pull request: "test: add test for specifying custom pidfile via `-pid`"
(https://github.com/bitcoin/bitcoin/pull/30724)
This small PR adds test coverage for the `-pid` command line option, which allows to overrule the pid filename (`bitcoind.pid` by default). One can specify either a relative path (within the datadir) or an absolute one; the latter is tested using `self.options.tmpdir`. Note that the functional test file `feature_init.py` so far only contained a stress test; with this new sub-test added, both the description and the test name are adapted to be more generic.
(https://github.com/bitcoin/bitcoin/pull/30724)
This small PR adds test coverage for the `-pid` command line option, which allows to overrule the pid filename (`bitcoind.pid` by default). One can specify either a relative path (within the datadir) or an absolute one; the latter is tested using `self.options.tmpdir`. Note that the functional test file `feature_init.py` so far only contained a stress test; with this new sub-test added, both the description and the test name are adapted to be more generic.
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1732784821)
hm... `sa_len=0` is really strange because it doesn't know how to advance, there isn't even place for the size! (which it just read in the line above it!) it should at least break out of the loop in that case
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1732784821)
hm... `sa_len=0` is really strange because it doesn't know how to advance, there isn't even place for the size! (which it just read in the line above it!) it should at least break out of the loop in that case
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1732832981)
i think i get it, we're advancing in the wrong way; see https://opensource.apple.com/source/xnu/xnu-1504.9.17/bsd/net/rtsock.c.auto.html (some actual Apple source code)
```
#define ROUNDUP32(a) \
((a) > 0 ? (1 + (((a) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t))
#define ADVANCE32(x, n) (x += ROUNDUP32((n)->sa_len))
...
dlen = ROUNDUP32(sa->sa_len);
m_copyback(m, len, dlen, (caddr_t)sa);
len += dlen;
...
```
ROUNDUP32:
- if the size is 0, advance by sizeof(uint32_t)
- o
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1732832981)
i think i get it, we're advancing in the wrong way; see https://opensource.apple.com/source/xnu/xnu-1504.9.17/bsd/net/rtsock.c.auto.html (some actual Apple source code)
```
#define ROUNDUP32(a) \
((a) > 0 ? (1 + (((a) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t))
#define ADVANCE32(x, n) (x += ROUNDUP32((n)->sa_len))
...
dlen = ROUNDUP32(sa->sa_len);
m_copyback(m, len, dlen, (caddr_t)sa);
len += dlen;
...
```
ROUNDUP32:
- if the size is 0, advance by sizeof(uint32_t)
- o
...
💬 hodlinator commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312554026)
> Also #28509.
Thanks for the added context @hebasto! There seems to some agreement that upgrading to Python 3.12 would [fix](https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1726482147) the issue. However, I was unable to find a documented fix, do you have a link? Your [comment](https://github.com/bitcoin/bitcoin/issues/28411#issuecomment-1724233900) links to an issue with `subprocess.run` but it's a higher-level function implemented on top of `subprocess.Popen` which we use.
>
...
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312554026)
> Also #28509.
Thanks for the added context @hebasto! There seems to some agreement that upgrading to Python 3.12 would [fix](https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1726482147) the issue. However, I was unable to find a documented fix, do you have a link? Your [comment](https://github.com/bitcoin/bitcoin/issues/28411#issuecomment-1724233900) links to an issue with `subprocess.run` but it's a higher-level function implemented on top of `subprocess.Popen` which we use.
>
...
💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312567392)
> There seems to some agreement that upgrading to Python 3.12 would [fix](https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1726482147) the issue. However, I was unable to find a documented fix, do you have a link?
This was just a wrong comment by myself. See https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1727885738
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312567392)
> There seems to some agreement that upgrading to Python 3.12 would [fix](https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1726482147) the issue. However, I was unable to find a documented fix, do you have a link?
This was just a wrong comment by myself. See https://github.com/bitcoin/bitcoin/pull/28509#issuecomment-1727885738
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2312583122)
i still intend to do this, but honestly i've kind of lost track of the scope. Can we first aim to fix the bugs here, then merge an implementation that at least doesn't have more problems than the current `libnatpmp` implementation?
Then later on we can make sure all edge cases with `bind`, `listenport`, routers assigning different ports, etc. are handled.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2312583122)
i still intend to do this, but honestly i've kind of lost track of the scope. Can we first aim to fix the bugs here, then merge an implementation that at least doesn't have more problems than the current `libnatpmp` implementation?
Then later on we can make sure all edge cases with `bind`, `listenport`, routers assigning different ports, etc. are handled.