< bitcoin-git>
[bitcoin] kostaz opened pull request #12570: Add test cases for HexStr (std::reverse_iterator and corner cases) (master...master) https://github.com/bitcoin/bitcoin/pull/12570
< bitcoin-git>
[bitcoin] 532479301 opened pull request #12573: Consensus: Fix bug when compiler do not support __builtin_clz* (master...vs2017) https://github.com/bitcoin/bitcoin/pull/12573
< pierre_rochard>
Is there an equivalent to ScriptToAsmStr for an CTxIn’s scriptWitness?
< pierre_rochard>
the current implementation of TxToUniv in core_write.cpp just uses HexStr
< wumpus>
pierre_rochard: I don't think so
< pierre_rochard>
Ok, I’ll settle for HexStr until I really need it, then I’ll try my hand at a PR :)
< pierre_rochard>
Thank you for the quick response wumpus!
< wumpus>
I'm not sure it's possible to unambigiously distinguish between data encodings there, so it's shown as just hex. But maybe something better could be done, not sure...
< bitcoin-git>
bitcoin/master f0e7aa7 Evan Klitzke: Add new prevector benchmarks....
< bitcoin-git>
bitcoin/master e46be25 Akio Nakamura: Reduce redundant code of prevector and speed it up...
< bitcoin-git>
bitcoin/master 5aad635 Evan Klitzke: Use memset() to optimize prevector::resize()...
< bitcoin-git>
[bitcoin] laanwj closed pull request #12549: Make prevector::resize() and other prevector operations much faster (master...prevector) https://github.com/bitcoin/bitcoin/pull/12549
< bitcoin-git>
[bitcoin] laanwj closed pull request #10387: Eventually connect to NODE_NETWORK_LIMITED peers (master...2017/05/node_network_limited) https://github.com/bitcoin/bitcoin/pull/10387
< Randolf>
provoostenator: Normally, when running any sort of a server daemon, sleep mode isn't typically wanted. After all, if the machine is in sleep mode, the daemon can't respond to any inbound queries.
< provoostenator>
Right, that's what I would expect too...
< Randolf>
provoostenator: There are some hardware configurations which can "wake up" the machine when network traffic is received, but since Bitcoin has a constant flow of data even this would in effect be pointless.
< Randolf>
...because the machine would constantly be waking up if ever it could get into sleep mode in the first place.
< provoostenator>
Maybe we can add --caffeinate as a flag (for OSX, maybe Linux has a similar thing)?
< Randolf>
provoostenator: I think it's better to leave the power management to the OS. I also think it's reasonable to assume that someone running a server daemon should also have enough knowledge to manage their power settings according to their needs.
< provoostenator>
Yeah, that's what I thought to. But apparently I'm not not knowledgeable enough. :-)
< Randolf>
If Bitcoin were to add options to manage power settings, then that would also be adding complexity to the project which I think would generally be beyond the scope of what Bitcoin is.
< Randolf>
And then whenever a vendor changes their power management API, precious development time gets taken away from more important things.
< provoostenator>
I probably made the mistake of logging out all users from the UI. It probably turned itself off once I closed the last SSH connection into the machine, despite bitcoind and a detached virtual box sessions.
< provoostenator>
I tend to agree bitcoind shouldn't do that for the OS. Though we could add some installation instructions / hints.
< Randolf>
provoostenator: I think that knowing how to change your system's power management configuration is something you'll have to master.
< Randolf>
Again, power management is, in my view, beyond the scope of what Bitcoin does.
< Randolf>
It could be good for a Wiki page.
< Randolf>
provoostenator: Consider this: Windows 10 regulatly reboots to install updates without asking the user first. Should Bitcoin's instructions tell users how to change the settings in Windows 10 to stop this from happening? And then update those instructions every time Microsoft changes the way
< Randolf>
And again, I think it's reasonable to assume that anyone who wants to run bitcoind is knowledgeable enough to manage their own OS accordingly.
< Randolf>
those settings are configured? I think this would also be beyond the scope of what Bitcoin does.
< zks_>
Guys, I'm new to Bitcoin Core source code... and I have a question - why base_blob::GetHex() reverses the base_blob::data[WIDTH] array?
< zks_>
Hope it is an appropriate place to ask questions...
< wumpus>
zks_: that's an old tradition, to represent hashes in reversed order
< zks_>
wumpus: Thanks for the answer! But why?
< wumpus>
zks_: we inherited it from satoshi and it's not realistic to change the entire interface
< zks_>
wumpus: Is it related to network byte order?
< wumpus>
no, definitely not
< mmgen>
zks_: possibly because it shows leading zeroes of the block header as *leading* zeroes
< zks_>
wumpus: Ok. Got it. Historic reasons...
< mmgen>
zks_: block header hash
< wumpus>
hashes are just blobs of data, their 'network byte order' is simply their memory representation
< wumpus>
there is no logic to it
< mmgen>
zks_: that's my reasoning on why Satoshi did it that way
< zks_>
mmgen: wumpus: If I get it right - there is no difference how to store the hashes (regular or reversed order). It just has to be this way or the other...
< wumpus>
I'm not sure GetHex on blob is the best place to do that swap, as it precludes using it in a neutral fashion, should probably ave been an external function GetWackyExternalRepresentation() but meh...
< mmgen>
zks_: this is how they're represented, not how they're stored
< wumpus>
indeed, internally the data is in normal order, just for external representation it's swapped
< wumpus>
makes sense to use the same conventions in your own software, swap it only for communication with bitcoind
< zks_>
so, in the blockchain "database" the hashes are always in the natural order and only printed in reverse?
< wumpus>
yep
< zks_>
ok, ... moving on guys... thx!
< wumpus>
satoshi's orignal vision: print hashes in reversed order :-)
< promag>
wumpus: not sure if this makes sense ^ but please comment since you wrote the origin code
< promag>
*original
< wumpus>
promag: no, I did not write that code, that came literally from the satoshi wxwindows code
< promag>
I'll update the PR description and add a before image later
< promag>
oh :P
< wumpus>
I think it's correct, though a bit weird
< promag>
it's weird to see the fee as debit in the first output
< wumpus>
the idea is that the total works out
< promag>
we could "hide" fee records by default with a checkbox
< promag>
right
< wumpus>
so satoshi's reasoning there is that it doesn't matter which of the outputs the fee is added to, but it must be added to one, otherwise the total doesn't match
< wumpus>
probably there's something similar in the RPC wallet
< Randolf>
(Sorry, I meant "merged" earlier rather than "closed.")
< wumpus>
12501 isn't on that list, should it be?
< Randolf>
I suspect that PR 12501 probably needs more peer-review and discussion.
< wumpus>
luke-jr: great!
< wumpus>
luke-jr: looks like jonasschnelli has some, unreplied to comments there
< wumpus>
it's fine to say that you're leaving them for a later PR, but please do reply to ereview comments
< Randolf>
Okay.
< kanzure>
hi.
< promag>
hi
< kanzure>
btw i am still seeking topic suggestions (either stuff you want to talk about, or you want other people to talk about) for next week's event.
< kanzure>
speaking of which, we should decide about next weekly meeting timing since i imagine some folks will be traveling
< wumpus>
#action send kanzure further topic suggestions
< wumpus>
right, I'll definitely not be there next week
< wumpus>
will be travellingback at that time
< kanzure>
wasn't aware we'd lose a bunch of people to fc18 but makes sense.
< promag>
regarding multiwallet, there are other details that can be left for other pulls
< wumpus>
indeed, apparently same problem this week
< sipa>
sorry!
< luke-jr>
sipa: next time, schedule FC so it doesn't conflict.
< sipa>
haha!
< luke-jr>
:p
< * Randolf>
laughs
< wumpus>
so I think we should skip next week's IRC meeting
< achow101>
ack
< Randolf>
Ack.
< kanzure>
we can move it forward if we want.. since a lot of folks in same room. but it's sort of redundant.
< wumpus>
right
< sipa>
sgtm
< luke-jr>
I suggest we have kanzure transcribe RL stuff to #bitcoin-core-dev in real time
< kanzure>
that's okay with me since roasbeef wont be there
< luke-jr>
lol
< kanzure>
(i love him tho)
< wumpus>
hehe
< btcdrak>
oh what did I miss?
< luke-jr>
1/4th of the meeting
< wumpus>
<luke-jr> I suggest we have kanzure transcribe RL stuff to #bitcoin-core-dev in real time
< wumpus>
<kanzure> that's okay with me since roasbeef wont be there
< wumpus>
not much is going on, everyone is at FC apparently
< luke-jr>
end early and spend 45 minutes on #11383 ? :D
< luke-jr>
Randolf: well, it'd be nice to get a few more utACKs first (although I've shipped 11383 in Knots so long that I doubt there's any problems to find left)
< achow101>
I'll take a look at 11383
< promag>
luke-jr: I'll review again
< Randolf>
wumpus: In PR 12501 an issue arose about the "virtual size" of the transaction. I'm thinking that it would probably be best to not mention this so as not to confuse end-users, but there's one person who's in favour of specifying this. If there's a link to documentation that can get into the
< Randolf>
"See also," not see all. :)
< Randolf>
details of the virtual size of the transaction, then I'm also thinking that including the link in the tooltip as a "see all" item should keep everyone happy?
< luke-jr>
Randolf: the value being configured is fundamentally tied to virtual size. I don't think it's avoidable.
< wumpus>
it's most important to be correct / complete
< luke-jr>
"adjusted size" might be more understandable in plain English
< luke-jr>
but there's no precedent for calling it that yet
< wumpus>
in general, even if certain terms might confuse users, it's better to mention something than leave it out and say the wrong thing
< wumpus>
but yeah, virtual size is confusing
< Randolf>
Okay. I want the tooltip to be correct without adding confusion.
< luke-jr>
weight-adjusted size?
< wumpus>
but calling it differently might be even worse
< wumpus>
I don't know
< wumpus>
(as you can't google it then!)
< luke-jr>
it's really a different way of speaking of the weight, not the size
< Randolf>
From a plain-English perspective, "weight-adjusted size" is much nicer, but that point about it being a new term is an important one because then it needs to be in the full documentation too.
< luke-jr>
I can't think up a nice way to call it
< Randolf>
Originally, I didn't have the word "virtual" in there.
< luke-jr>
I suggest we just stick to "virtual size" until some English genius thinks up a better name
< wumpus>
if there are new terms there's a good rationale to only use a single term for it, not make up multiple terms just because they sound nicer
< Randolf>
I agree.
< wumpus>
so if it is virtual size, I think we need to bite the bullet and simply use that
< Randolf>
sipa: I wonder if it might be helpful to have a glossary of terms included in a separate file.
< wumpus>
it would be something to put in the UI user documentation, if we had any :)
< Randolf>
I suppose bits and pieces could be put together at first, and then later formed into a proper document.
< Randolf>
I guess there would be two documentation files -- one for bitcoind, and the other for the GUI.
< Randolf>
Or perhaps they should just be separate chapters to avoid overlap.
< rex_4539>
Are you interested in a PR about fixing all the potential typos in the source code? I could go through all the source code, fix all typos, and squash to a single commit. If you are interested, let me know if you want me to fix all typos (comments and readable messages) or just a particular type. I have already fixed them in Zcash and Monero projects.
< Randolf>
rex_4539: How does one fix "potential typos?"
< Randolf>
Fixing existing typos is always welcome.
< rex_4539>
Very tedious work, I can assure you :) Not a developer but willing to help in any capacity I can.
< Randolf>
That's fantastic.
< Randolf>
My suggestion is this...
< Randolf>
Do all the corrections in documentation in one PR, and do all the corrections in source code in another PR.
< Randolf>
That is, if you want to keep the number of PRs down to a minimum.
< Randolf>
You're also welcome to break them apart into smaller PRs if you like.
< Randolf>
One of the advantages of smaller PRs is that it's easier to get them reviewed and merged.
< rex_4539>
The good thing with typos is that they are extremely trivial to review. Shouldn't take more than 5 minutes for the whole Bitcoin project.
< Randolf>
Your contributions are most certainly welcome. :)
< rex_4539>
Do you want me to fix also comments or just readable messages?
< Randolf>
rex_4539: That's entirely up to you as this is a volunteer effort after all.
< jcorgan>
PRs with changes that are scattered through the code, like typo fixes, can sometimes make other PRs need rebasing (if merged)
< rex_4539>
I'm asking because each team works differently. Zcash team was happy to have all fixed, Monero wanted just readable comments.
< rex_4539>
*messages
< Randolf>
rex_4539: Oh, I see. All fixes are welcome.
< rex_4539>
Don't worry, I will not touch typos in the actual code, just comments and readable messages.
< Randolf>
jcorgan: Does it make a difference if the changes are in a very small number of very large PRs, or if the changes are in a lot of smaller PRs?
< rex_4539>
In my experience in the 2 previous projects, they wanted one big BR because each separate PR was using the CI machines all the time :P
< jcorgan>
i think there's a way to comment your PR so it gets ignored by CI, don't remember
< jcorgan>
appropriate for doc only changes, but not for code comments, as you might accidentally break syntax
< Randolf>
Oh, I'd like to know how to comment my PR to prevent the CI checks. That would reduce system resource load then.
< jcorgan>
but i wouldn't worry so much about it now, i'd focus on getting the work done and submit a single PR for it, then if it needs breaking into multiple ones it will come out in the review
< rex_4539>
I think there is a rollup command for the bot.
< Chris_Stewart_5>
isn't travis 'free' for open source projects?
< Randolf>
I still think it would be good to separate documentation updates from code/comment updates into two PRs though, at least.
< jcorgan>
free, but capacity limited and might only run one job at a time
< jcorgan>
agree on code/comment and doc separation
< Randolf>
Chris_Stewart_5: For doc-only PRs that I've been submitting, if I knew I could do something to prevent unnecessary resource usage, I would have been happy to do it.
< rex_4539>
Sure, 2 separate PRs it is then. Will get to it in a couple of days.
< Randolf>
rex_4539: Wow, thank you!
< jcorgan>
another general purpose consideration for pull requests is that independent things don't get combined into one request, as the maintainer can't selectively merge one or the other
< jcorgan>
anyway, just some thoughts from experience as a maintainer elsewhere, carry on :-)
< rex_4539>
Yes, but maintainer can comment on the ones he disagrees and I can revert those commits.
< jcorgan>
in general, the less work the maintainer and reviewers need to do, the more likely it is to get merged
< Randolf>
Chris_Stewart_5: Apparently the [ci-skip]"
< Randolf>
Chris_Stewart_5: Apparently, including the text "[ci-skip]" (without the quotation marks) in a PR's message will prevent Travis-CI from attempting a build.
< Randolf>
rex_4539: You may want to include this text in your doc-only PRs to prevent the build process: [ci-skip]
< Randolf>
rex_4539: When changing comments, you should still let the build occur.
< Randolf>
Someone in the #travis channel just indicated to me that it's this: [ci-skip]
< Randolf>
If they all work (which would be good), then it doesn't matter.
< Randolf>
I suggest using the in-comment style that in PR #7658 instead of including it in the PR's title -- keeping those titles simple is generally encouraged, and they're also limited in size anyway as far as I know.
< bitcoin-git>
[bitcoin] dooglus opened pull request #12580: Show a transaction's virtual size in its details dialog (master...show_vsize) https://github.com/bitcoin/bitcoin/pull/12580