< fanquake>
Doing a binary comparison of a bitcoin-qt built from HEAD~1 and HEAD~2, building on Debian using depends. Currently seeing this diff: https://gist.github.com/fanquake/653bb42176d7772578db08a0f8e60f11 . Any suggestions as to what could be causing the difference? bitcoind matches.
< fanquake>
The change in the src between the two is only in Python test code, so that should be it.
< fanquake>
*shouldn't
< wumpus>
fanquake: do you happen to know what section this difference is in?
< wumpus>
e.g. if it's in .text it might be useful to look at the disassembly
< fanquake>
wumpus Ok. I'm just rebuilding, but assume the same diff will happen again, can check that for you shortly.
< wumpus>
was about to ask that: if you get this difference without any C code changes, then, I wonder if you do get a stable output running it on the same commit again and again
< fanquake>
Hopefully we'll know that shortly 🔍
< wumpus>
if the difference is only in -qt it could suggest non-determinism in one of the qt tools
< wumpus>
i don't think i've ever used build-for-compare with bitcoin-qt, at all
< wumpus>
running the same compare now
< fanquake>
wumpus: testing the new depends --prefix as well?
< fanquake>
I've just done master (b7fbf74b980ebb122ae34b142f2cc49b44b92de3) and a dummy commit, still seeing the same difference in bitcoin-qt.
< fanquake>
Looks like the difference is in libbitcoinqt_a-qrc_bitcoin_locale.o
< fanquake>
.rodata._ZL18qt_resource_struct
< wumpus>
no, not using --prefix at the moment, I see a diffrence too between the same commits though, in bitcoin-qt but not bitcoind
< bitcoin-git>
bitcoin/master c5d3787 Andrew Chow: Allow createwallet to take empty passwords to make unencrypted wallets
< bitcoin-git>
bitcoin/master 6841b01 MeshCollider: Merge #16394: Allow createwallet to take empty passwords to make unencrypt...
< bitcoin-git>
[bitcoin] meshcollider merged pull request #16394: Allow createwallet to take empty passwords to make unencrypted wallets (master...fix-born-enc) https://github.com/bitcoin/bitcoin/pull/16394
< wumpus>
fanquake: my guess would be: timestamp metadata in the compiled resource data
< fanquake>
meshcollider: If your merging, #15986 probably ready as well.
< fanquake>
However if I'm using Qt from depends then that should be included ?
< fanquake>
eh right, QT_RCC_SOURCE_DATE_OVERRIDE wont have been set etc
< wumpus>
(i was not not using qt from the depends for my comparison, just ubuntu 18.04's system one)
< coinmonks>
Hey Guys, I am writing an article around Bitcoin codebase activity, anyone wanna look it and give me some feedback..
< wumpus>
fanquake: but yes, going to test the --prefix option next
< fanquake>
wumpus: no worries. I'm going to rebuild while exporting that ENV var, and i assume it'll fix the Qt issue. If so I'll probably open a PR to change it to be exported by default in depends.
< wumpus>
coinmonks: maybe link it here then people can look if they're interested
< coinmonks>
warning - English is my second language..
< coinmonks>
I am online if anyone have any feedback,,or they can just leave private notes on the post itself.. thank you every one for contributing on Bitcoin.. :]
< fanquake>
wumpus: yep QT_RCC_SOURCE_DATE_OVERRIDE fixed the issues with bitcoin-qt 🤦
< jonasschnelli>
emilengler: I think if you pass "-resetguisettings" at startup you'll get a backup .ini file in your datadir...
< jonasschnelli>
(that maybe helps if you want to inspect)
< emilengler>
And where is the config file initial be loaded? In src/qt/bitcoin.cpp or src/qt/intro.cpp
< jonasschnelli>
emilengler: I think whenever it touches QSettings
< jonasschnelli>
mainly qt/optionsmodel.cpp
< jonasschnelli>
certainly when there is .setValue() or value()
< jonasschnelli>
First "read" is probably in GetLangTerritory()
< bitcoin-git>
[bitcoin] dongcarl opened pull request #16519: guix: Change manifest to use channels and inferiors (master...2019-06-guix-channels-and-inferiors) https://github.com/bitcoin/bitcoin/pull/16519
< jonasschnelli>
I can't attend at todays meeting (swiss national day and some fam. duties).
< jonasschnelli>
If someone wants to pickup my. topic (bitcoin-dev mailing list moderation), feel free
< jonasschnelli>
I propose that we add more moderators to shorten the moderation lag which has been between >24h, thus makes debates cumbersome
< jonasschnelli>
Eventually there are some volunteers for moderation, ideally neutral people
< bitcoin-git>
[bitcoin] Remagpie opened pull request #16521: wallet/rpc: Use the default maxfeerate value as BTC/kB (master...maxfeerate-as-rate) https://github.com/bitcoin/bitcoin/pull/16521
< dongcarl>
Serialization question: in an `Unserialize`, is it possible to do something like this: `s >> static_cast<uint8_t>(m_network_id);`? Or do I have to split this up? `m_network_id` is an `enum class` backed by `uint8_t`
< sipa>
dongcarl: i belive static_cast<uint8_t&>(m_network_id) will work
< * dongcarl>
trying
< sipa>
seems not
< dongcarl>
yeah... "invalid static_cast from type ‘NetworkID’ to type ‘uint8_t&’"
< sipa>
though you can use `uint8_t x; s >> x; m_network_id = x;`
< dongcarl>
sipa: Yeah I was using that before, just thought there might be something more elegant haha
< sipa>
dongcarl: in my (long outdated) serialization rework #10785 i have a "READWRITEAS(type, value)"
< sipa>
which would let you just write READWRITEAS(uint8_t, m_networkid) for both serialization and deserialization
< dongcarl>
sipa: That can still be used if we're not using the `SerializationOp` magic?
< sipa>
actually i suspect it won't work here; references need to be convertible for this
< dongcarl>
`s >> *(uint8_t *)&m_network_id;` worked
< dongcarl>
which is... cool i guess
< sipa>
dongcarl: i'm not sure that's legal
< sipa>
s >> *static_cast<uint8_t*>(&m_network_id) does not work
< sipa>
you can always access the byte representation of other objects, which means it's not UB to do this, but i'm not convinced it's guaranteed to have the desired effect
< dongcarl>
sipa: Oh it's a reinterpret cast of some kind?
< sipa>
yeah, it's a reinterpret cast
< sipa>
i suspect that the representation of a class enum is defined to be equal to its underlying int type, which would make this correct
< sipa>
but i'm not entirely sure
< dongcarl>
Oh well, better to have multiple lines than to be unsure about safety :-) Will probably get optimized by the compiler anyway
< sipa>
yes
< achow101>
dongcarl: does `static_cast<uint8_t>(m_network_id)` not work?
< dongcarl>
achow101: noop :-/
< achow101>
you can have `uint8_t id; s >> id; static_cast<NetworkID>(id);
< achow101>
just have a uint8_t temp variable
< sipa>
achow101: easier is `uint8_t id; s >> id; m_network_id = NetworkID(id);`
< dongcarl>
yup, we're going the temp variable route, sipa didn't know you could do `NetworkID(id)`, neat!
< jnewbery>
I've just pushed to 16060. It's ready for rereview
< jnewbery>
(thanks for the review, aj!)
< wumpus>
aj: yes, maybe for the best, having two competing PRs open is usually not very productive
< achow101>
It seems like #16341 has Concept ACKs, so maybe move it to blockers? At least isn't labeled with "needs conceptual review" anymore
< gribble>
https://github.com/bitcoin/bitcoin/issues/16341 | Introduce ScriptPubKeyMan interface and use it for key and script management (aka wallet boxes) by achow101 · Pull Request #16341 · bitcoin/bitcoin · GitHub
< wumpus>
I think 7 blockers is enough :)
< wumpus>
otoh doesn't seem you have one yet there
< achow101>
it got merged :)
< wumpus>
ok moving it then
< provoostenator>
I'd love to build on top of The Box, so not opposed to making it high prio.
< wumpus>
at least #16363 is almost, or entirely ready for merge, I think
< BlueMatt>
do people think this should be signed or unsigned in rust-bitcoin
< wumpus>
well, other implementations could make it unsigned, if that makes the code easier
< BlueMatt>
like, if its unsigned, people get confused reading crap from rpc
< BlueMatt>
if its signed, people may misimplement CSV
< sipa>
no strong opinion either way; if people want to change it, i think this is fairly easy to review for correctness
< wumpus>
rust-bitcoin is not consensus critical, so the amount at stake for an implementation error is somewhat less their
< MarcoFalke>
[15:18] <sipa> one easy way to make sure you have all the call sites is to rename it
< BlueMatt>
right
< provoostenator>
I can confirm nVersion is a source of confusion :-)
< BlueMatt>
the background is someone got confused parsing rpc output or something similar, and wants to change the unsigned nVersion to signed
< provoostenator>
1 is the same signed and unsigned?
< wumpus>
that only gets the direct usage sites though, it's somewhat harder to analyse where the value ends up indirectly
< BlueMatt>
anyway, enough discussion, its somewhat minor...in 5 seconds everyone say their prefernce and we'll flip a weighted coin based on the response and close or not :p
< MarcoFalke>
+0.001
< aj>
MarcoFalke: you're voting for signed floating point? :)
< BlueMatt>
aj: no, signed Decimal
< elichai2>
BlueMatt: make it signed and cast when pass to libconsensus? lol
< sdaftuar>
how about we cast to unsigned in the rpc handler
< wumpus>
^^
< sdaftuar>
and then stop thinking about it for a long time
< BlueMatt>
sounds fine to me too
< sipa>
sgtm
< wumpus>
exactly, if it confuses people in RPC, then report it differently in RPC :)
< BlueMatt>
cool, next topi
< MarcoFalke>
sdaftuar: Doeparsers decode 32bits to signed?
< BlueMatt>
c
< MarcoFalke>
Oh, json doesn't use bits
< wumpus>
#topic any contributors affected by GH blocking access/functionality in certain countries? (fanquake)
< BlueMatt>
well whats the eta until auzzies cant work on core cause their govt forces them ato add backdoors and gh kicks them out?
< BlueMatt>
do we need to get fanquake a freedom visa?
< achow101>
and aj
< BlueMatt>
right
< moneyball>
Nat tweeted saying it only affects private repos. I'm not sure if that matches reality or not.
< wumpus>
from what I've heard, currently it shouldn't be a problem because Iran/Crimea/etc is only locked out of their private repos
< provoostenator>
BlueMatt: Microsoft gladly added a backdoor to Skype for China, so I don't think they'll kick anyone out.
< wumpus>
but in the longer run it's not clear what will happen
< emilengler>
Has someone a list of the countries who are blocked?
< emilengler>
Or regions
< wumpus>
emzy: right, getting the source code isn't hard, but losing access to PRs/issues etc to be able to contribute back would be bad
< achow101>
emilengler: it's in the help.github article I linked earlier
< achow101>
emilengler: Crimea, Cuba, Iran, North Korea, and Syria.
< sipa>
afaict, the only thing we should be discussing here now is whether we should prioritize figuring out in what ways our processes are dependent on github
< wumpus>
yes
< emzy>
right
< dongcarl>
I know that a few of the depends packages depend on other GitHub repos
< wumpus>
I think our process is already kind of detached from github in a way: we dont use it for merging, a lot of us prefer reviewing locally, the ACK system could work everywhere, etc
< sipa>
yeah, i think if worst comes to worst, we can spin up something else
< sipa>
it'd be annoying, but not devastating
< achow101>
the annoying part is losing the issues and PRs
< wumpus>
the good part you mean
< wumpus>
*ducks*
< sdaftuar>
lol
< moneyball>
ha
< meshcollider>
Lol
< wumpus>
just file the ones that matter again :p
< elichai2>
unless we think of this ahead of time and start slowly duplicating all of github into a private gitlab (we could "fake" the PRs and issues to be the same as in github if it's an open source platform)
< moneyball>
can't we just export issues/PR data on a regular basis as backup?
< wumpus>
seriously though, maybe there's some way to import gh metadata
< BlueMatt>
I presume as long as we (a) have very good backups of the entire pr/issue/everything context and (b) are willing to switch upon seeing any actual real-world issues for people, then I think we dont need to do anything today, no?
< wumpus>
moneyball: we do!
< moneyball>
oh nice!
< MarcoFalke>
So people from those countries can create pull requests and issues, right?
< dongcarl>
Is there a GitHub<->GitLab mirroring tool that keeps them in sync?
< sipa>
MarcoFalke: afaict, the only thing is access to their private repos
< achow101>
MarcoFalke: seems like it
< phantomcircuit>
BlueMatt, visa doesn't change an australian citizens obligation to backdoor stuff for their government
< emilengler>
GitLab can import github issues/pr as well
< phantomcircuit>
just cant trust those convicts anymore
< wumpus>
hehe
< wumpus>
phantomcircuit: discrimination!
< wumpus>
anyhow, not much to say on this topic I think, no one from those countries spoke up at least
< BlueMatt>
ehh, if openbsd can discriminate against us citizens for the same reason, I think we're allowed to discriminate based on a convict colony
< wumpus>
(if you are from those countries feel free to PM me)
< sipa>
BlueMatt: you know the difference between a cup of yoghurt and australia?
< achow101>
oh no
< dongcarl>
something something culture?
< wumpus>
that leaves one topic "bitcoin-dev mailing list moderation", which I'm kind of scared of and jonasschnelli isn't here anyway
< sipa>
is warren or kanzure here?
< phantomcircuit>
wumpus, in all seriousness the first part is actually true, the obligation is of citizens and residents, not merely of people in australia
< achow101>
I think it was just a question about whether we had enough mailing list moderators
< emilengler>
How does the list moderation works? It is slow that's the only thing I know..
< wumpus>
"I propose that we add more moderators to shorten the moderation lag which has been between >24h, thus makes debates cumbersome"
< sipa>
arguably the ML isn't really on topic here, as it's not a bitcoin core thing
< wumpus>
"Eventually there are some volunteers for moderation, ideally neutral people"
< wumpus>
yea exactly...
< sipa>
plus it doesn't seem that any of the list operators are here now
< MarcoFalke>
Could the mailing list be used for this discussion?
< sipa>
yes
< MarcoFalke>
ok, endmeeting :)
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Aug 1 19:40:36 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< bitcoin-git>
[bitcoin] metalicjames opened pull request #16523: Add removemempoolentry RPC to evict transactions from the mempool (master...removemempoolentry) https://github.com/bitcoin/bitcoin/pull/16523
< fanquake>
I’m almost glad I didn’t turn up for that meeting
< fanquake>
Sounds like I should cancel my Aussie citizenship ASAP heh
< bitcoin-git>
[bitcoin] jtimon opened pull request #16524: Truly decouple wallet from chainparams for -fallbackfee (master...b19-true-wallet-no-chainparams) https://github.com/bitcoin/bitcoin/pull/16524
< bitcoin-git>
[bitcoin] TheBlueMatt closed pull request #16513: [RFC] Switch CTransaction::nVersion to an unsigned integer (master...2019-07-unsigned-tx-ver) https://github.com/bitcoin/bitcoin/pull/16513
< bitcoin-git>
[bitcoin] TheBlueMatt opened pull request #16525: Dump transaction version as an unsigned integer in RPC/TxToUniv (master...2019-07-unsigned-tx-ver) https://github.com/bitcoin/bitcoin/pull/16525
< bitcoin-git>
[bitcoin] jtimon opened pull request #16527: Get rid of Params().RequireStandard() (master...b19-chainparams-no-requirestd) https://github.com/bitcoin/bitcoin/pull/16527