< bitcoin-git>
[bitcoin] benthecarman opened pull request #15185: docs: Spelling error fix on fuzzing.md (master...docs_fuzzing_spelling_mistake) https://github.com/bitcoin/bitcoin/pull/15185
< gkrizek>
wumpus the problem is the 'encoding' arg to the subprocess.check_output function. That was introduced in 3.6. I don't really see a need for that parameter anyway, so I think it's safe to just remove it. I can open a PR for it
< wumpus>
gkrizek: the encoding parameter is for operating systems such as FreeBSD which don't set the system locale to utf-8 by default, leaving it up to python to decide what the locale is. It'll then pick ASCII only which results in problems in some cases.
< wumpus>
gkrizek: (for example when calling git, and any of the commit messages contains characters >=128)
< bitcoin-git>
[bitcoin] fanquake opened pull request #15186: rpc: remove duplicate solvable field from getaddressinfo (master...duplicate-solvable-fields) https://github.com/bitcoin/bitcoin/pull/15186
< wumpus>
promag: from the point of view of our API that's true, though on the other hand there's nothing in JSON that disallows multiple values per key
< wumpus>
hebasto: sure
< wumpus>
promag: and checking that *efficiently* would involve adding a set to the Univalue type; not instead, but in addition to the vector because we want to preserve order as well
< promag>
true, univalue is generic and that's fine. just think it could have more specialized mutations. for instance, pushKV should be setKV ?
< wumpus>
(also asserting is really dangerous here; it can turn a mild asthetic issue into a crash)
< promag>
wumpus: that should be fine, we don't use user keys
< wumpus>
(say, REST returns some JSON structure and the code can be manipulated to add the same field twice, somehow, whoopsie instant DoS - a better solution would be to replace the value for the existing key)
< wumpus>
that'd also be "javascript semantics" FWIW, in any case there's no need to overreact to this case
< wumpus>
if there's anything to worry that would be that this should have been caught at *review time* :)
< promag>
I'm not arguing it shouldn't be caught at review time
< wumpus>
I know, but I mean *if* this should trigger any kind of discussion it's that; the end result of a key appearing twice on the API is hardly a problem, the only worry (if this was, say, a silent merge issue) is that it could have been worse
< wumpus>
*silent merge conflict*
< fanquake>
wumpus how often do you use the mallocinfo mode for getmemoryinfo ?
< fanquake>
wumpus just curious, don't see it mentioned often, and had just about forgotten it was a thing, but am writing RPC wrappers. Can't use it on macOS anyways.
< wumpus>
it's mostly useful for developers when diagnosing some kinds of allocation behavior
< dongcarl>
luke-jr: how does OpenRC work with env vars? How are they passed to the service?
< dongcarl>
I can do the same for #12255, which means the user can override env vars in their systemd unit
< bitcoin-git>
[bitcoin] practicalswift opened pull request #15187: fees: Complete the removal of fee-estimation file read code for old versions (master...fee-estimation) https://github.com/bitcoin/bitcoin/pull/15187
< wumpus>
fanquake: so it might be a bug in their packaging of the new version, instead of the code itself?
< jnewbery>
promag: I've lost the thread a bit on where we are with #13100. Is there a path to getting all the load/unload/create wallet functionality into the GUI for v0.18?
< gleb>
I would appreciate if we prioritize 14897 because a) I know someone stacks new changes on top of it and b) people on twitter are really exciting about replicating the topology inference through this vuln. on mainnet :)
< jonasschnelli>
#14897
< gribble>
https://github.com/bitcoin/bitcoin/issues/14897 | randomize GETDATA(tx) request order and introduce bias toward outbound by naumenkogs · Pull Request #14897 · bitcoin/bitcoin · GitHub
< sipa>
luke-jr: i'm not sure that's possible; do you see anything obvious that could be left out?
< sipa>
or gleb ?
< luke-jr>
sipa: well, the original PR was apparently 60 LOC changed, and it got revised by request?
< gleb>
Half of the new LOC is a comment :)
< gleb>
Most of the comments on the original work were about *significant* refactoring (move from net to net_processing), which I did
< luke-jr>
you mean insignificant?
< wumpus>
if you got those comments and implemented them I think it's unfair to complain about it now
< luke-jr>
I'm not sure I have the original code to compare
< luke-jr>
if it was just a code move, no worries
< luke-jr>
gmaxwell's suggestion sounded more in depth than that though
< achow101>
luke-jr: github shows the diffs between force pushes now
< luke-jr>
achow101: how?
< luke-jr>
ooh, neat
< wumpus>
ok, any other topics?
< luke-jr>
rebasing seems to break it though
< meshcollider>
achow101: only in a nice way if it was a commit amendment, rebases are impossible to read
< gleb>
luke-jr: Well, I first did what gmaxwell suggested (before other reviews), THEN received refactoring comments, and then we moved code. Not sure which step was wrong and what I should've done better :)
< bitcoin-git>
[bitcoin] practicalswift closed pull request #15187: fees: Complete the removal of fee-estimation file read code for old versions (master...fee-estimation) https://github.com/bitcoin/bitcoin/pull/15187
< luke-jr>
gleb: I'm not saying any of it was wrong, just that doing it in two separate steps/PRs would make it easier to backport ONLY the fix part
< wumpus>
if people deem the refactor a necessary part of this, then that should be backported too
< sipa>
mostly afk, but will check occasionally if someone pings me
< wumpus>
but sure if it's possible to do a minimal fix for the 0.17 branch that might be less risky, if this is a risky refactor, but if it's move-only I don't think that's the case
< wumpus>
for master this is fine anyhow
< wumpus>
any other topics?
< wumpus>
apparently not! that's a short meeting then
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Jan 17 19:21:25 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< sipa>
gwillen: IANAL but i believe those copyright statements per file (and much less the year indications) are almost certainly worthless, especially when attributing to "The Bitcoin Core developers", which is not a legal entity
< wumpus>
it wouldn't prevent people from opening such PRs anyway
< wumpus>
and just posting 'we do this at the end of the year' then closing isn't exactly that much work
< gwillen>
sipa: I am in total agreement with you on this
< gwillen>
right, but we do still actually do it, right?
< wumpus>
yes, once per year with an automated script
< gwillen>
ahh, *nods*
< gwillen>
removing all the years would prevent the PRs and having to run the script, but if it's fully automated I guess it could be worse.
< wumpus>
hah yes, that's true, removing the years would make it impossible for people to make PR changing them
< gwillen>
also like, when I made a new file that was largely code copied from another file, I spent some time contemplating what exactly I should do for the copyright header
< bitcoin-git>
[bitcoin] practicalswift opened pull request #15191: validation: Add missing cs_LastBlockFile locks in PruneAndFlush() and UnloadBlockIndex(). Add missing locking annotation for nLastBlockFile and fCheckForPruning. (master...cs_LastBlockFile) https://github.com/bitcoin/bitcoin/pull/15191
< gwillen>
which would be fixed by having a generic invariant one.
< luke-jr>
gwillen: copyright counts even if there's no copyright notice at all
< luke-jr>
IANAL also btw
< gwillen>
yeah, this is true. I was going to say that a written copyright notice still does something, but after quickly reading the wikipedia article on copyright notices to refresh my memory... it's not actually clear that they have any practical effect here
< [\\\]>
According to copyright.gov, "Copyright notice is optional for works published on or after March 1, 1989, unpublished works, and foreign works; however, there are legal benefits for including notice on your work."
< gwillen>
the strongest benefit that Wikipedia lists is that it prevents an infringer from claiming ignorance as a defense, which would reduce statutory penalties
< wumpus>
also mind that this is an international project, not only US law counts
< gwillen>
anyway I'm not advocating removing the notice
< luke-jr>
arguably, we can't remove it entirely due to the MIT license and Satoshi's notices
< gwillen>
just replacing it with some kind of simple fixed notice with a pointer to the COPYING file, without varying years or names other than "the Bitcoin developers" or what have you
< wumpus>
IIRC the idea was to remove the years, not the entire notice
< gwillen>
yeah, I would advocate for removing anything that changes between files