< promag> review/merge beg #13339
< gribble> https://github.com/bitcoin/bitcoin/issues/13339 | wallet: Replace %w by wallet name in -walletnotify script by promag . Pull Request #13339 . bitcoin/bitcoin . GitHub
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/94c0287aec9a...263f53e2d07a
< bitcoin-git> bitcoin/master fad027f MarcoFalke: scripted-diff: Add missing spaces in RPCResult, Fix type names
< bitcoin-git> bitcoin/master 263f53e MarcoFalke: Merge #18098: scripted-diff: Add missing spaces in RPCResult, Normalize ty...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18098: scripted-diff: Add missing spaces in RPCResult, Normalize type names (master...1912-rpcDocFixes) https://github.com/bitcoin/bitcoin/pull/18098
< bitcoin-git> [bitcoin] luke-jr opened pull request #18165: Consolidate service flag bit-to-name conversion to a shared serviceFlagToStr function (master...svcflags2str) https://github.com/bitcoin/bitcoin/pull/18165
< jonasschnelli> Oh. I can macOS notarize the 0.19.1rc2 binary
< jonasschnelli> "message": "The executable does not have the hardened runtime enabled.",
< fanquake> jonasschnelli: likely need some additions to a .plist somewhere
< jonasschnelli> Probably... strange it worked with rc1?!
< fanquake> Interesting
< jonasschnelli> maybe they (Apple) just started to enforce it? dunno
< fanquake> I think that might be the case, was just trying to find the dates
< fanquake> It would have been January when you signed rc1 right? https://developer.apple.com/news/?id=09032019a
< fanquake> I assume now that it's Feb something has changed
< jonasschnelli> I notarized rc1 5 days ago and Sjors did confirm it worked: https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-585324648
< jonasschnelli> Interesting:
< jonasschnelli> Apple had just a warning for the rc1 for the hardening entitlements
< fanquake> hmm. I can PR the .plist requirements if you're not already working on it
< jonasschnelli> fanquake: Yes. Please do if you know how...
< jonasschnelli> There is eventually a flag we need to change during code signing...
< jonasschnelli> (just looking into it)
< jonasschnelli> OTHER_CODE_SIGN_FLAGS
< jonasschnelli> I can't see a difference in the Info.plist when enabling hardening in a sample XCode project..
< jonasschnelli> I guess its more on the code signing level?
< fanquake> I think the changes might actually be in a separate .xml file
< fanquake> Which you pass to the codesigning tool
< jonasschnelli> fanquake: I think so. Yes.
< fanquake> Although we can add that to our tree with the other macdeploy files
< jonasschnelli> fanquake: I guess we need `--options runtime` during codesign
< jonasschnelli> (detached-sig-create)
< jonasschnelli> fanquake: would be nice if you can investigate further and PR. We can make a testbuild for rc2 and make sure we land in rc3 or final
< fanquake> Sure. I can take a look at some changes.
< wumpus> FWIW I'm not going to be at bitcoin 2020 / coredev next month, sorry, even less eager than normal to go on a long flight like that with the coronavirus and quarantaine scare
< jonasschnelli> I won't be there as well...
< wumpus> I don't have a good feeling about this tbh
< promag> wumpus: keep calm and enjoy merging #13339
< gribble> https://github.com/bitcoin/bitcoin/issues/13339 | wallet: Replace %w by wallet name in -walletnotify script by promag . Pull Request #13339 . bitcoin/bitcoin . GitHub
< wumpus> yes these are the times i'm quite happy to work remotely
< wumpus> jonasschnelli: glad to see the notarization worked
< fanquake> wumpus: If you want to get even more remote, feel free to come hang out on the farm. hah.
< wumpus> fanquake: thinking about it :)
< bitcoin-git> [bitcoin] laanwj pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/263f53e2d07a...051439813e2c
< bitcoin-git> bitcoin/master 9a5b5ee Joao Barbosa: wallet: Replace %w by wallet name in -walletnotify script
< bitcoin-git> bitcoin/master 4e9efac Joao Barbosa: test: Check wallet name in -walletnotify script
< bitcoin-git> bitcoin/master 0514398 Wladimir J. van der Laan: Merge #13339: wallet: Replace %w by wallet name in -walletnotify script
< promag> \o/
< promag> wumpus: I was kidding... that PR is really bad
< bitcoin-git> [bitcoin] laanwj merged pull request #13339: wallet: Replace %w by wallet name in -walletnotify script (master...2018-05-walletnotify) https://github.com/bitcoin/bitcoin/pull/13339
< wumpus> worst PR ever
< promag> sorry heh
< wumpus> happy we got it over with though, that was one shedpainting party
< promag> curious about who is going to pick win support
< promag> not me!
< promag> #18160
< gribble> https://github.com/bitcoin/bitcoin/issues/18160 | gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged by promag . Pull Request #18160 . bitcoin/bitcoin . GitHub
< promag> this one might fix high cpu of bitcoin-qt
< wumpus> me neither, linux and BSD only here
< promag> so if you have a big wallet test let me know
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/051439813e2c...179504ccb6f4
< bitcoin-git> bitcoin/master 530d02a fanquake: build: pass -fno-ident in Windows gitian descriptor
< bitcoin-git> bitcoin/master 179504c Wladimir J. van der Laan: Merge #17948: build: pass -fno-ident in Windows gitian descriptor
< bitcoin-git> [bitcoin] laanwj merged pull request #17948: build: pass -fno-ident in Windows gitian descriptor (master...pass_fno_ident) https://github.com/bitcoin/bitcoin/pull/17948
< promag> I wasn't aware of MODEL_UPDATE_DELAY = 250ms
< promag> so we are calling GetBalances 4 times a second
< promag> *always*
< wumpus> at least there should be a try_lock there so that lock contention won't hang the whole UI, but yea
< promag> don't get fooled with that try_lock
< promag> not waiting for locks is nice obviously
< wumpus> right it helps a bit but doesn't change that it's still bad to do that polling in the GUI thread
< promag> problem is after, if GetBalances takes some time, and also it's called for no good reason
< promag> right
< promag> also we are caching the last block height but we aren't checking for that as early as possible
< wumpus> mind that the balance can change without the block height changing
< wumpus> (e.g. if you send, or unconfirmed balance for unconfirmed incoming transactions)
< wumpus> then again that's always after a transaction
< promag> yeah, there's the force flag for that
< wumpus> I guess the polling is only needed if blocks changed
< wumpus> (e.g. coinbases becoming mature, transactions confirming)
< promag> and if locks couldn't be acquired
< promag> I'm planning to move the timer from walletmodel(s) to wallet controller, and move the calls to the background thread
< wumpus> makes sense
< promag> but that's a bigger change and I think 18160 improves a lot cpu usage and is an easy backport
< promag> let's see what jonasschnelli and ryanofsky say
< luke-jr> wumpus: does RISC-V need #17569 backported? (why not?)
< gribble> https://github.com/bitcoin/bitcoin/issues/17569 | build: Allow export of environ symbols and work around rv64 toolchain issue by laanwj . Pull Request #17569 . bitcoin/bitcoin . GitHub
< wumpus> luke-jr: couldn't hurt at least
< luke-jr> wumpus: well, it means a rc3... :/
< luke-jr> wumpus: if the rc2 binaries work on RISC-V as-is, then we don't need it in 0.19.1 at least
< luke-jr> (IIRC you have a RISC-V system to test on, right?)
< luke-jr> (to be clear, it's the noexecstack commit that I'm unsure about - obviously gitian builds complete ;P)
< hebasto> promag: mind looking into #17966 ?
< gribble> https://github.com/bitcoin/bitcoin/issues/17966 | qt, refactor: Optimize signal-slot connections logic by hebasto . Pull Request #17966 . bitcoin/bitcoin . GitHub
< dongcarl> What's the current thinking on running Bitcoin Core on non-ECC devices? Are there enough safe guards in place so that at least casual users can use non-ECC devices?
< sipa> what is a non-ECC device?
< sipa> elliptic curve crypto?
< sipa> error correcting code?
< dongcarl> devices without ECC RAM haha
< luke-jr> I suspect most people do
< luke-jr> IIRC Intel doesn't even allow you to use ECC with their normal CPUs
< rafalcpp> dongcarl: almost all users sadly sit on non-ECC RAM. and you have no idea what will bitflip, it could be disk driver that will erase wallet or whatever. probably rather pointless to defend in software
< sipa> is there *any* consumer oriented CPU that supports ECC?
< rafalcpp> sipa: pretty sure AMD Bulldozer supports, for example
< luke-jr> rafalcpp: there are places we *should* definitely defend.. your backups won't help you if you bitflip a change address
< luke-jr> sipa: POWER9
< luke-jr> (not typical, but the 4-core variants are consumer-oriented)
< dongcarl> I guess I'm just trying to understand the specific risks here...
< dongcarl> If I want to buy hardware to be my dedicated Bitcoin Core node... Should ECC RAM be a main concern? Or is it outweighed by other concerns
< luke-jr> dongcarl: I would put supervisor-free over ECC
< dongcarl> luke-jr: supervisor = BMC?
< luke-jr> dongcarl: nah, backdoors like Intel ME and AMD PSP
< sipa> dongcarl: if an ECC system is more expensive than a non-ECC system that can run two instances of bitcoind in parallel, pick the latter :)
< luke-jr> and the Microsoft ThreadX in Raspberry Pi
< rafalcpp> this purity in choosing realiable and not backdoored hardware is highly inspiring at least for me
< dongcarl> sipa: That's very true!
< dongcarl> luke-jr: Is that normally reported on wikichip? e.g., how do I tell if https://en.wikichip.org/wiki/socionext/sc2a11 has a supervisor?
< * dongcarl> googles Microsoft ThreadX
< rafalcpp> 2-of-2 multisign on other, cheap, device would solve it basically, although much less comfortable to use, dongcarl. an idea for future when it becomes non-trivial amount
< luke-jr> dongcarl: I don't know :/
< dongcarl> rafalcpp: Right, I think the fact that usable SBCs are getting cheaper and cheaper is great. However, setting up clusters is quite a pain still
< dongcarl> Another question: When testing out lower-power SBCs, what's a good metric for "runs Bitcoin Core well"? Is "being able to keep up with tip" good enough?
< luke-jr> dongcarl: IBD in an hour? :P (haha)
< yevaud> dongcarl: even on the upper end SOCs IBD takes days.
< yevaud> dongcarl: the cheapest "sbc" with ECC is almost certainly the APU2/APU4, it's still slow and fanless, like any other SBC.
< dongcarl> Disregarding IBD I mean... In a steady state, is there anything else that would hinder correct operation and security other than keeping up with tip?
< dongcarl> yevaud: right, from my understanding the apu2 might be affected by PSP, but I'm not sure that the NICs will cooperate if someone wants to attack
< yevaud> not really. on most SBCs you're going to see rather high rates of data loss due to MicroSD cards, but they run acceptably in the sync state.
< dongcarl> luke-jr: Did you get to look at the apu2 architecture and evaluate the risks closely?
< rafalcpp> dongcarl: a cluster? just run two separate instances. If anything is needed, I suppose bitcoind/gui could support 2-2 multisign more easily (if it doesn't yet)
< yevaud> dongcarl: yes, it has a PSP core.
< luke-jr> dongcarl: no
< luke-jr> dongcarl: I saw AMD and closed it
< dongcarl> yevaud: Right, but what's the exact attack though? From my understanding you'd need the NICs to cooperate, and PSP isn't in the coreboot payload that PC Engine builds...
< dongcarl> rafalcpp: That's true, you could probably do some deduplication at the FS level to save some space too
< rafalcpp> dongcarl: well this isn't separate computer then. but just fully prune one (or both) then it is around 6 GB
< yevaud> dongcarl: I don't know without asking PcEngines (do, they're helpful), but I don't think it's a realistic concern in the real world.
< luke-jr> I'm not sure PcEngines would know either?
< dongcarl> yevaud: That's a good point. Will email.
< dongcarl> luke-jr: They have quite a deep understanding of their hardware: https://github.com/pcengines/apu2-documentation
< luke-jr> dongcarl: but afaik the PSP is inivislbe outside the CPU?
< yevaud> luke-jr: on the Intel one it's stored on an external flash chip.
< luke-jr> but at least on Intel, if you deprive it, it won't work
< yevaud> stands to reason the manufacturer would have to understand that, if it were the case on AMD.
< * dongcarl> has some reading to do
< luke-jr> yevaud: they might not understand the ramifications of not providing it, even if that is the case
< luke-jr> eg, on Intel the IME is used to workaround silicon bugs, so with me_cleaner and such, you're vulnerable to undisclosed vulnerabilities
< yevaud> uh, really? the microcode is a separate thing to the ME core.
< bitcoin-git> [bitcoin] practicalswift opened pull request #18166: ci: Run fuzz testing test cases under valgrind (master...fuzz-test-cases-under-valgrind) https://github.com/bitcoin/bitcoin/pull/18166
< luke-jr> yevaud: someone disclosed or leaked that a while ago. it's not just microcode that patches silicon issues.
< luke-jr> I don't know that any of the specific details of the silicon bug were disclosed, so we can only speculate on why Intel might have done ti that way
< luke-jr> (maybe it's a silicon issue with the ME core, and not patching it means external attackers can compromise the ME core?)
< dongcarl> (if this is off-topic, someone should let me know)
< dongcarl> luke-jr: I think the PSP firmware has to be in the BIOS to be loaded
< yevaud> luke-jr: I was under the impression that ME operations against the main cores were effectively a NMI, so having it do any sort of continuous operation would demolish performance.
< dongcarl> And in the apu2 case, they did not include it in the coreboot payload: https://coreboot.coreboot.narkive.com/7e2UHc2x/pc-engines-apu2-platform-psp-support
< yevaud> luke-jr: but obviously I don't have specific knowledge here, so I'll leave it at that.
< luke-jr> yevaud: I'm not sure why that would matter. Injecting a bit of code wouldn't need to be continuous, and most things can be done by reading/changing RAM directly
< wumpus> luke-jr: they work fine without the patch, #17569 does work around a noexecstack issue (so the stack is executable), I don't think it's worth doing a new rc for but if there is one anyway we could include it
< gribble> https://github.com/bitcoin/bitcoin/issues/17569 | build: Allow export of environ symbols and work around rv64 toolchain issue by laanwj . Pull Request #17569 . bitcoin/bitcoin . GitHub
< dongcarl> Does anyone know how to setup gribble for another channel? I'm thinking #bitcoin-builds
< bitcoin-git> [bitcoin] TheQuantumPhysicist opened pull request #18167: Fix a violation of C++ standard rules where unions are used for type-punning (master...master) https://github.com/bitcoin/bitcoin/pull/18167
< achow101> dongcarl: talk to nanotube