bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<hebasto>
rc3?
ghost43_ has quit [Remote host closed the connection]
ghost43 has joined #bitcoin-core-dev
brunoerg has quit [Ping timeout: 250 seconds]
<willcl_ark>
Should I be able to use the `redeemScript` from a `bitcoin-cli createmultisig 1-of-4` as the signet challenge for a custom signet? Struggling to get it working like this...
Kaizen_K_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]
<_aj_>
willcl_ark: should start with 51 and end with 54ae? yeah, should work.
ghost43 has quit [Remote host closed the connection]
<willcl_ark>
_aj_: hmmm thanks.It does indeed start and end with those chars
ghost43 has joined #bitcoin-core-dev
<_aj_>
willcl_ark: (they're "1" and "of 4 CHECKMULTISIG" respectively)
<willcl_ark>
Right! The error thrown by bitcoind when trying to mine (on a machine which only has one of the private keys imported) is "Specified sighash value does not match value stored in PSBT"
<_aj_>
willcl_ark: last time i tried, i had difficulty generating a private key then importing it into a wallet that i could use with the signet it was signing for -- wallets use the network magic to stop you from using your mainnet wallet on testnet, but the network magic for signet depends on the signet challenge
ghost43 has quit [Remote host closed the connection]
<_aj_>
willcl_ark: update to newer master / 23.x rc, fix for that was merged a few hours ago i think
<_aj_>
not 23.x rc, 23.x branch; hasn't been a new rc for it yet i think
<fanquake>
so is that literally "signet is broken"
<laanwj>
sigh
<fanquake>
hebasto: does it just stop after "[0%]..." ?
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<hebasto>
in 50% cases
<hebasto>
in another 50% of cases it fails immediately
<laanwj>
that's really bad
<sipa>
That's bizarre.
<fanquake>
definitely only happens for cross-compiled windows?
<MarcoFalke>
Another compiler bug?
<hebasto>
^^ that is I can say
<laanwj>
either that or undefined c++ behavior
<laanwj>
which ahppens to be compiled correctly on MSVC but faulty in mingw, but yeahh who knows, if there's not any kind of error
<MarcoFalke>
Does it crash in gdb.exe as well?
<laanwj>
might try to compile with hardening disabled (to test, not to release), it has been a source of compile bugs on windows
<laanwj>
i don't have windows i could try in wine though
<lightlike>
hebasto: does that also happen when syncing signet fresh from genesis? In the log linked in 24726, you seems to use an existing datadir synced to 99%.
AaronvanW has quit [Quit: Leaving...]
<hebasto>
lightlike: removed signet dir, and it fails immediately appr. in a half of cases
<fanquake>
I'm trying to recreate. cross-compiled master and running bitcoind.exe -signet under wine
<fanquake>
haven't seen a crash yet
<lightlike>
oh - i had no problems syncing signet with win 10, but will try more times if it happens only sometimes
<hebasto>
fwiw, currently testing on Windows 11 Pro 21H2 virtual machine
<laanwj>
the report was for rc2 not master (if that makes the difference it would be interesting ofc)
<laanwj>
slightly differnt guix env maybe
<laanwj>
or maybe it's not a problem in wine but only on windows 11 that points even more at a possible issue with hardening features
<fanquake>
have downloaded the rc2 bins to test under wine
<fanquake>
ok, got a crash
<fanquake>
wine: Unhandled page fault on read access to FFFFFFFFFFFFFFFF at address 000000014051CC65 (thread 003f), starting debugger...
<laanwj>
happy to see it reproduces
<laanwj>
and that it's apparently fixed on master 9?)
<laanwj>
anything to add, remove, or that is in the list and almost ready for merge?
<luke-jr>
#24294 should probably be in the list
<gribble>
https://github.com/bitcoin/bitcoin/issues/24294 | RPC: Switch getblockfrompeer back to standard param names blockhash and nodeid by luke-jr · Pull Request #24294 · bitcoin/bitcoin · GitHub
kvaciral[m] has joined #bitcoin-core-dev
<laanwj>
luke-jr: added, though you have two in the list now
<luke-jr>
oops, wrong list. I meant the 23.0 list
<luke-jr>
sorry
<laanwj>
ok
<jonatack85>
luke-jr: i argued against that change in my review feedback there but agree there's something to be said for the principle of least surprise. so ~0 but in that case the developer notes about rpc argument naming should maybe be clarified
brunoerg has quit [Remote host closed the connection]
<laanwj>
i've added it to 23.0 milestone, no opinion on the change really
<jonatack85>
there's maybe also a question of naming consistency within an rpc, versus across rpcs
<luke-jr>
I kinda see it as 23.0 or not worth the effort, myself
<laanwj>
but agree that if it is to be changed back it needs to be before the release
<laanwj>
otherwise it's another breaking change
kvaciral[m] has left #bitcoin-core-dev [#bitcoin-core-dev]
Talkless has quit [Quit: Konversation terminated!]
<laanwj>
anything else? any other topics?
<jeremyrubin>
hi
kvaciral[m] has joined #bitcoin-core-dev
<laanwj>
there's something to be said for standardizing parameter names across RPCs, it's kind of annoying as user if every RPC has slightly different inconsistent spelling for the same thing
<laanwj>
it's also a lot of work and may result in breaking changes in itself if persued too obsessively, so it's more of a best effort within the other constraints thing
<jeremyrubin>
i am indifferent -- i don't think we have strong enough consistency in the RPCs right now that one more inconsistency hurts, if we want things to be really strongly consistent we should fix it all up in a big breaking API changes release. the worst case, IMO, is subtle small changes that might not get caught. better to just have nothing work than
<jeremyrubin>
for things to seem to work.
<jeremyrubin>
otoh, this seems like a good one since it's a reversion of an unecessary change
<glozow>
hi
<laanwj>
jeremyrubin: right, for new RPCs it makes sense to choose a parameter name already used by other RPCs for a same thing instead of trying to come up with something cleverer
<laanwj>
i definitely agree absolute consistency is not a goal let that be clear
<laanwj>
anyhow, any other topics?
gribble has quit [Remote host closed the connection]
<laanwj>
fanquake: i suppose it also happens on rc3?
gribble has joined #bitcoin-core-dev
<fanquake>
laanwj: not sure. doing a cross compile of master with mingw-w64 and gcc 10.3 to match guix, and see if it still happens
<fanquake>
i would assume it would though
brunoerg has joined #bitcoin-core-dev
<laanwj>
that's a good thing to try
brunoerg has quit [Ping timeout: 260 seconds]
<fanquake>
can't make that crash
Guest5843 has quit [Quit: Client closed]
<laanwj>
interesting, so there's really something different on master
<fanquake>
i couldn't make the self-compiled 23.x, built with GCC 9 crash either, only the release binaries so far, so I'm leaning towards something in guix
<hebasto>
^ agree
<laanwj>
trying freshly built 23.0rc3 bitcoind.exe -signet in wine now
<laanwj>
it's at block 1000, when is it supposed to crash
<hebasto>
at start
<hebasto>
try to stop and re-run it
<laanwj>
ok-seems that guix rc3 doesn't have the problem, then
<laanwj>
oh it's at restart?
yanmaani has quit [Remote host closed the connection]
<hebasto>
yes
<laanwj>
it doesn't crash here
<hebasto>
with `-signet`, right?
<laanwj>
yes
yanmaani has joined #bitcoin-core-dev
<hebasto>
hmm, our guix environment has no significant changes between rc2 and rc3
An0rak has joined #bitcoin-core-dev
<laanwj>
same with rc2, no crash
<laanwj>
wine-7.4 (staging) fwiw
<laanwj>
let's try with ubuntu 22.04 standard wine
<laanwj>
ok: wine 6.0.3, fresh environment, let rc2 sync to block 1000 again, restarted, no crash
<laanwj>
restarted about 10 times, no crashes, i'm afraid i cannot help reproduce here
brunoerg has joined #bitcoin-core-dev
bytes1440000 has joined #bitcoin-core-dev
<bytes1440000>
I had some issues with rc2 on windows. It was crashing and I had to restart 10 times to finally use it (bitcoind and bitcoin-qt). Could not find anything in logs. Never had this issue with any release in past.
<laanwj>
could it depend on the processor architecture ?
<bytes1440000>
Ignored because it could have been a system specific issue or something wrong with windows.
<bytes1440000>
Can troubleshoot or provide more logs if someone can guide me.
<laanwj>
anyhow fanquake's report has a memory map we shoudl be able to find out where it crashes
<luke-jr>
laanwj: bytes1440000 is talking about the same issue?
<laanwj>
luke-jr: could be?
<bytes1440000>
luke-jr: I am not sure
<laanwj>
rebuilding rc2 guix to get the debug information
<laanwj>
as it happens semi-randomly it could be some kind of race condition, in worst case threading is broken in the guix windows env
vysn has quit [Ping timeout: 260 seconds]
bytes1440000 has left #bitcoin-core-dev [#bitcoin-core-dev]
<laanwj>
wait… is DEBUG_LOCKORDER on for the guix build?
<laanwj>
i'm seeing "Enter: lock contention m_mutex, ./checkqueue.h:…" early in -debug output
<jonatack85>
it's no longer a build option, it's a runtime one
<laanwj>
seems like heavy overhead to always be doing that, i don't expect it to be the cause of the issue ofc
<jonatack85>
the "lock" logging category
<laanwj>
so for every lock it first does try_lock then lock?
<laanwj>
even with lock debugging disabled
<jonatack85>
yes
<sipa>
that sounds expensive; was the impact of that benchmarked?
<fanquake>
It certainly looks like there was never really any benchmarking done? It's mentioned in the thread a number of times
<laanwj>
you didn't gate the try_lock on the debug category as i suggested
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<jonatack85>
initially yes, it did
<laanwj>
i really think we shouldn't mess with the good-weather case (simply calling ::lock) if there's no reason not to
<sipa>
thinking about how it's likely implemented (but i don't know exactly), i suspect that a succesful lock and succesful try_lock are very similar in performance
<laanwj>
our synchronization wrappers probably inline quite a lot of overhead to every lock call in any case
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]
<sipa>
which would mean that a change like this mostly hurts in case there is contention, not so much in happy cases
<laanwj>
it's unnecessary though
<sipa>
yes
<jonatack85>
sipa: that was my conclusion
<sipa>
i'd still like to see benchmarks...
brunoerg has quit [Remote host closed the connection]
<cfields>
jonatack85: right, I was complaining about the contention tracking itself there, not the specific test failure.
<jonatack85>
yes, it was an issue with DEBUG_LOCKCONTENTION defined as well
<laanwj>
even it it's unlikely to cause any issues, complicating the locking default case seems risky
<laanwj>
jonatack85: yes, but that was only enabled for debugging, not in production
<jonatack85>
the lock logging on by default in the unit tests only revealed it
<laanwj>
most users won't want to debug lock contention and don't need that overhead, so it makes sense to disable it in the release builds
<cfields>
right, the only reason I can think of that we'd want it on for release builds is if we intended to actually use it for decision making: if (contended_time > x), which would be a really nasty idea :)
<laanwj>
now that is the kind fo cursed idea i like to hear ! :)
<jonatack85>
is the suggestion is to remove it from releases while leaving as a runtime option on master?
brunoerg has joined #bitcoin-core-dev
<sipa>
laanwj: Don't give anyone ideas, please!
<laanwj>
jonatack85: i think it should be a compile-time option, which is disabled unless something is passed to --configure, i don't mind if it's also gated by a runtime option or not
larpsus has left #bitcoin-core-dev [#bitcoin-core-dev]
larpsus has joined #bitcoin-core-dev
<larpsus>
sipa: lol
<jonatack85>
cfields: might compile quicker, iiuc? DEBUG_LOCKCONTENTION took a long time to build and couldn't be turned off at run time, impractical to use for development
larpsus has left #bitcoin-core-dev [#bitcoin-core-dev]
<laanwj>
eeeeeekkkkkkk
<cfields>
jonatack85: eh, it's been a while and I've largely forgotten the details now. I'll just stay out of it and review a follow-up PR if something arises :)
<laanwj>
fanquake: 0x000000014051cc65: sha256d64_avx2::Transform_8way(unsigned char*, unsigned char const*) at /distsrc-base/distsrc-23.0rc2-x86_64-w64-mingw32/src/crypto/sha256_avx2.cpp:316
<laanwj>
this explains why i can't reproduce it; i don't have a avx2 capable CPU
<cfields>
OoOooh
gnaf has quit [Quit: Konversation terminated!]
<sipa>
You may need an AVX2 capacable CPU which does not have SHA-NI.
<sipa>
*capable
<jonatack85>
laanwj, cfields: ok
<sipa>
@laa
<cfields>
jonatack85: just had another look, yeah, looks like I moved the defines out of the header so that you only have to recompile 1 object instead of everything, and also lends itself to runtime switching. IIRC the only real downside was lost inlining for some lock operations.
<sipa>
laanwj: Do you know exactly what instruction it's failing on?
<sipa>
It wouldn't surprise me that the lost cost of inlining is worse than what is gained by doing lock instead of trylock+lock.
<sipa>
(because the happy path of trylock and lock both on x86 is just a memory fetch)
<jonatack85>
cfields: nice. I'll start with reverting for now and happy to review if you take that further.
<sipa>
> Unhandled page fault on read access to FFFFFFFFFFFFFFFF at address 000000014051CC65
<cfields>
sipa: I don't disagree about the cost, but there's so much fluff in our locking I think there's ~0 chance it currently compiles down to a fetch.
<sipa>
Easy to find out.
<laanwj>
sipa: vmovdqa %ymm4, 0x260(%rsp)
<sipa>
heh, that's just copying a variable from the stack
<sipa>
so... stack underflow?
<cfields>
sure that's the crashing thread?
<laanwj>
could it be a misalignment or something
<laanwj>
misaligned stack pointer, it would explain the chance element
brunoerg has joined #bitcoin-core-dev
<laanwj>
nah there's other vmovdqa instructions in the function as well, why would it choose this one to crash
<sipa>
this is in the output producing part of Transform_8way
<sipa>
the only memory that should be accessing is the output
brunoerg has quit [Ping timeout: 272 seconds]
<sipa>
which it can't assume is aligned
<laanwj>
the other vmovdqa instructions involve other registers or rip-relative addressing, this is the first stack relative one
<sipa>
But why would it be copying anything to the stack in the first place. It should be accessing the output.
<sipa>
And it's not an alignment problem I think... 0xFFFFFFFFFFFFFFFF actually sounds like a bogus address being addressed.
<laanwj>
i don't know it's likely a compiler bug, we haven't seen this on other platforms
<laanwj>
i don't know enough about wine error reporting to know if that message is really showing an address or it doesn't know what to do with avx2 instructions
<sipa>
Fair.
<cfields>
fanquake: ^^ how's that clang+mingtw work coming along? :)
<cfields>
*mingw
<laanwj>
getting 0xFFFFFFFFFFFFFFFF exactly on a %esp+0x260 access would be pretty unlikely as well :)
<sipa>
Yeah, indeed.
<laanwj>
i think it's writing to a stack temporary (spilling)
<sipa>
The x86_64 ABI requires %sp+8 to be 16-byte aligned on entry/exit of functions.
<laanwj>
the ABI defines the stack should be 16-byte aligned on windows at function calls, is this enough? ymms are 256 bits? but i don't know the alignment requirements
<laanwj>
oh, %sp+8?
<sipa>
ymms are 256 bits and require 16-byte alignment.
<sipa>
oh, or maybe even 32-byte alignment
<laanwj>
it pushes %r12, %rbx, then subtracts 0x338 from %rsp
<sipa>
-mavx2 code expecting higher alignment of stack pointers, e.g.
<laanwj>
but i think we found the compiler bug: it runs out of registers, spills to the stack using vmovdqa, and forgets it cannot make a 32-byte alignment assumption
<laanwj>
right
<sipa>
right, that sounds like a plausible explanation
<laanwj>
i think we'll want to avoid using 256 bit types on windows
<sipa>
that looks like it could be it
<sipa>
easy to just #ifdef out the avx2 logic on win
shesek__ has quit [Remote host closed the connection]
<laanwj>
at least for mingw, apparently MSVC is ok
shesek__ has joined #bitcoin-core-dev
<sipa>
may just be that the avx2 detection or compilation doesn't work in the first place on msvc?
<sipa>
(but sure, no reason to expect this bug exists on MSVC as well)
brunoerg has joined #bitcoin-core-dev
<laanwj>
right, i don't know
brunoerg has quit [Ping timeout: 245 seconds]
<cfields>
laanwj: that gcc bug report you linked mentions that it's not a problem if __m256i's are passed around via const ref, which we don't do.
<cfields>
I'm not suggesting we make that change, but we might be able to use it to verify if we're affected by that bug or not.
<laanwj>
please
<laanwj>
oh to verify, yeah sure
<laanwj>
but i really don't want some brittle workaround, it's not that important
rex4539 has quit [Remote host closed the connection]
<laanwj>
newer systems will have SHANI and others will fall back to some other optimized sha256 implementation
<jonatack85>
cfields: this one? "As a workaround, one can either use __attribute__((always_inline)) for *all* the functions accepting __m256 or pass *all* arguments by const-ref. Const-ref arguments are passed correctly."
<laanwj>
cfields: i also don't think that considers register spilling
rex4539 has joined #bitcoin-core-dev
bomb-on has quit [Quit: aллилѹіа!]
bomb-on has joined #bitcoin-core-dev
<sipa>
the shani code uses only 128bit registers afaik
rex4539 has quit [Remote host closed the connection]
<cfields>
jonatack85: right
bomb-on has quit [Ping timeout: 246 seconds]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] laanwj opened pull request #24727: build: Disable AVX2 code path on mingw builds (master...2022-03-windows-noavx2) https://github.com/bitcoin/bitcoin/pull/24727
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
brunoerg has joined #bitcoin-core-dev
<cfields>
laanwj: from your asm dump it looks like all of those calls are inlined already anyway, so agreed that wouldn't change the spilling.
<laanwj>
sipa: yep, also verified that
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
<reardencode>
Is there a way in PSBT to encode a position or ordering of signatures, or is it expected that the finalizer will (if needed for a particular script) do O(n^2) sig verifications to get the sigs in the right order (when order matters)?
surveillancechai has joined #bitcoin-core-dev
<surveillancechai>
bitcoin, privacy, censorship, monero etc. love that altcoiners still would comeback, contribute and use bitcoin if bitcoin developers care about privacy at protocol level
<sipa>
@reardencode It can just look at the order of keys in the script?
surveillancechai has left #bitcoin-core-dev [#bitcoin-core-dev]
<laanwj>
cfields: right, the problem doesn't arise with function arguments in our case
<reardencode>
@sipa "just look at the order" meaning try verify(sig, pubkey) for each sig for each pubkey?
<laanwj>
cfields: it's kind of disappointing, if they'd use the non-aligned store it'd maybe fractionally slower but at least not crash
<sipa>
@reardencode No. The script has pubkeys in a particular order. The PSBT has a map from keys to sigs. The finalizer just needs to emit the sigs in the order implied by their corresponding keys.
<reardencode>
@sipa ah, thank you! I was not seeing that the partial signatures are a map!
<cfields>
laanwj: yeah, i was just reading up on -mavx256-split-unaligned-store, thinking toggling that on/off might change the heuristic used to decide on aligned/unaligned. But even if it did work, that'd just be more voodoo to rely on.
Victorsueca has quit [Ping timeout: 260 seconds]
<laanwj>
cfields: it's intersting that such a flag exists at all
rex4539 has joined #bitcoin-core-dev
<sipa>
my entirely uninformed guess is that that flag wouldn't do anything, because this is a store which GCC (for better of worse) considers to be aligned
<laanwj>
maybe gcc got 'smarter' about alignment in newer versions explaining why it turns up now
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
mudsip has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
hashfunc159a has joined #bitcoin-core-dev
rex4539 has quit [Ping timeout: 240 seconds]
Kaizen_Kintsugi_ has quit [Ping timeout: 252 seconds]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
bomb-on has joined #bitcoin-core-dev
bomb-on has quit [Ping timeout: 260 seconds]
upekkha has quit [Quit: upekkha]
upekkha has joined #bitcoin-core-dev
shesek_ has joined #bitcoin-core-dev
shesek__ has quit [Remote host closed the connection]
Victorsueca has joined #bitcoin-core-dev
mudsip has quit []
Kaizen_Kintsugi_ has quit [Remote host closed the connection]
Kaizen_Kintsugi_ has joined #bitcoin-core-dev
Kaizen_Kintsugi_ has quit [Ping timeout: 260 seconds]