< wumpus>
jonasschnelli: I don't think that will work; you just moved settings.value(strSettingsVersionKey) to the end
< wumpus>
jonasschnelli: what do you have against what I suggested there? settingsVersion = settings.contains(strSettingsVersionKey) ? settings.value(strSettingsVersionKey) : 0; upfront, then use then from there on
< wumpus>
now I was confusing the settings names... this seems so trivial, but actually it's trivial to accidentally mess up
< jonasschnelli>
yes. Needs to focus to get this right...
< wumpus>
which is difficult with the heat
< wumpus>
my plan is to at least test 8371 today
< wumpus>
jonasschnelli: yes so if (settingsVersion < 130000 && settingsVersion == 100) also doesn't work, it makes the same confusion I did. We need to look at nDatabaseCache here :)
< jonasschnelli>
hah. To dump...
< jonasschnelli>
if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache"))?
< jonasschnelli>
.toInt() == 100 at the end
< wumpus>
if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toInt()==100)
< wumpus>
yes I think so
< wumpus>
or toLongLong() as we seem to provide it as qint64
< jonasschnelli>
Yes. Probably better,... though I don't expect machines with that much RAM. :)
< wumpus>
I don't either, but I'm not certain how qt handled type conflicts there. If it works with toInt() that's good enough :)
< wumpus>
okay thanks for updating, will re-test again soon, now looking at 8389
< wumpus>
you've certainly been solving many 0.13.0rc1 issues quickly, where would we be without you
< wumpus>
looking for more review for: Prevent fingerprinting, disk-DoS with compact blocks #8408
< jonasschnelli>
Yes. 8389 is more important
< jonasschnelli>
Looking at 8408
< jonasschnelli>
8408 makes sense. Props to sdaftuar! It needs extrem focus to find these issues...
< wumpus>
yes these are very sneaky, good to have them discovered and fixed before the release
< wumpus>
ooh, regtest has the same RPC default port as testnet. I only discover this now
< jonasschnelli>
wasn't aware of that...
< jonasschnelli>
Ah. Just the p2p port is differnt...
< wumpus>
me neither. Never tried to start a regtest node on the same host that was running a testnet node before
< wumpus>
it gives you: "Error: Unable to start HTTP server. See debug log for details." (where the log indeed contains details about failed binding)
< wumpus>
not a big issue just passing rpcport now, but I think it would make sense to bump that port number
< wumpus>
I get a "Warning: Error reading wallet.dat! All keys read correctly, but transaction data or address book entries might be missing or incorrect." with #8389, not sure why
< wumpus>
after that it just continues
< wumpus>
this is an old-fashioned, pre-HD wallet
< jonasschnelli>
let me have a look
< jonasschnelli>
wumpus: hmm.. that error should not be related to 8389 i guess
< wumpus>
let me see if it happens without
< wumpus>
maybe it's just a corrupted wallet, that's possible
< wumpus>
oops happens without #8408 too
< wumpus>
eh without 8389 I mean
< jonasschnelli>
hmm... this error means you had an error during ReadKeyValue() but not for a key
< wumpus>
moving wallet out of the way, creating a new non-HD wallet
< jonasschnelli>
I think your wallet.dat has a key that your current code cannot understand
< jonasschnelli>
yes. Maybe do a clean start
< wumpus>
that works fine, also after restarting bitcoind with that wallet
< wumpus>
so it was something specific to the previous wallet, maybe it was corrupted in some older testing. Ok, phew
< wumpus>
I've saved that wallet, might take a look at what key confused it later
< GitHub76>
[bitcoin] laanwj closed pull request #8389: [0.13] Create a new HD seed after encrypting the wallet (0.13...2016/07/hd_enc) https://github.com/bitcoin/bitcoin/pull/8389
< GitHub88>
bitcoin/0.13 f142c11 Jonas Schnelli: [0.13] Create a new HD seed after encrypting the wallet
< GitHub88>
bitcoin/0.13 de45c06 Jonas Schnelli: [Wallet] Add CKeyMetadata record for HDMasterKey(s), factor out HD key generation
< GitHub88>
bitcoin/0.13 c3c82c4 Wladimir J. van der Laan: Merge #8389: [0.13] Create a new HD seed after encrypting the wallet...
< jonasschnelli>
reminder: 8389 needs an "up-port" (my mistake)
< wumpus>
seems to apply cleanly to master, apart from the release-notes change which we can ignore there
< jonasschnelli>
Is this still required? Lost track of the osx based deployment process...
< jonasschnelli>
If not, I can close the PR
< cfields>
jonasschnelli: let me take another look. pretty sure i walked away from that one to think on it a bit. all solutions are ugly :\
< jonasschnelli>
heh.. sure.. I don't really care. I think deploying (dmging) on OSX is not something we need to support.
< jonasschnelli>
Depends/gitian based deployment should be sufficient IMO
< cfields>
jonasschnelli: iirc macports/brew use it though
< jonasschnelli>
Ah. Okay. that could be...
< jonasschnelli>
Yes. Not sure if 8135 fixes it completly... I know it worked on my local mac when I tested it a couple of weeks ago
< cfields>
jonasschnelli: what do you think of just pulling the ~3 python libs directly into support/macdeploy?
< cfields>
i believe we patch them all already
< cfields>
(until upstream merges the fixes, that is)
< jonasschnelli>
Or pull in a script that downloads and patches them?
< jonasschnelli>
I don't want to blow up our code base...
< cfields>
ok. thinking on it. i'll ack/pr something either way so we can move on
< cfields>
jonasschnelli: hmm, now that i think about it, macports/brew only use the .app and not the dmg. how about for osx we stop there, and only linux/gitian creates the dmg?
< cfields>
yes, they just use 'make appbundle'. so that works for me if it's ok by you
< jonasschnelli>
cfields: yes. I think this would make sense.
< jonasschnelli>
You don't need a dmg if you selfcompile
< jonasschnelli>
Only if you want to distribute... Which we should not encourage over non gitian
< cfields>
right
< jtimon>
is there any tutorial for monkies like me to do the gitian builds for a given release?
< sdaftuar>
sipa: i am having a hard time parsing your comment in #8408
< sipa>
sdaftuar: in general, peers should not send getblocktxn for anything near the tip
< sipa>
*anything not near the tip
< sdaftuar>
right, agreed
< sdaftuar>
are you suggesting that we disconnect for old blocks as wel?
< sdaftuar>
rather than not disconnect for unknown blocks?
< sipa>
if they do, they're either confused (and they're better off with us disconnecting them, or they'll wait long before timing out) or trying to do something nasty (in which case we're better off disconnecting them)
< sipa>
the question is whether there are no cases under which this can happen accidentally
< sdaftuar>
i assumed naively that this could happen on testnet
< sdaftuar>
ie without much rigorous thought
< sdaftuar>
mostly i guess i was comparing the handling of getblocktxn to how we handle getdata requests
< sdaftuar>
where we silently ignore unknown, and non-main-chain old blocks
< sipa>
ok, in that case let's follow the same approach
< sipa>
i think there is a lot of behaviour that's ad hoc and hard to reason about there, but at least that follows existing tested code
< sipa>
but perhaps we should rethink this in a bigger rework-block-sync-logic in the future...
< sdaftuar>
fyi i did add a test in #8418 that catches the behaviors corrected in #8408
< sdaftuar>
but 8418 is so much code that i didn't think it necessarily made sense as a bugfix PR for 0.13.0
< sipa>
agree
< sipa>
wait; 8418 has no changed behaviour, apart from what is already in 8408?
< sipa>
i assume
< sdaftuar>
it has one changed behavior, the new command line option -bip9params
< sdaftuar>
i needed a way to turn off segwit on regtest
< sipa>
sure sure
< sipa>
i mean it should no impact on anything in mainnet
< sdaftuar>
yes, agreed. it's a lot of testing code, plus something that should be regtest-only
< sdaftuar>
assuming i didn't screw up :)
< * luke-jr>
won't be able to make the meeting today
< jonasschnelli>
Yes. Its not an release not only PR
< morcos>
luke-jr: i don't understand why we need to change anything now. segwit is not released. using blockmaxcost is fine for 0.13
< gmaxwell>
s/not/note/
< luke-jr>
jtimon: blockmaxweight rsther than blockmaxsize
< morcos>
uh, or weight
< jonasschnelli>
gmaxwell: thanks. :)
< luke-jr>
wumpus: because people were insisting the code had to change to remove the recommencaation
< gmaxwell>
I think it's foolish that we're still releasing with settings that don't reflect the near ubiquitious network usage.
< sipa>
luke-jr: we shouldn't be relying on miners picking safe policy that only harms themselves
< jtimon>
gmaxwell: on that note, agree with sipa that we should decouple the consensus flags bit poisitions from the script flags', but that's only for master
< sipa>
luke-jr: if we're not fine with larger blocks, we shouldn't do segwit as-is
< morcos>
sipa: agreed
< morcos>
where we is the big we
< sipa>
in practice, nearly every miner will set the max block size and weight to the maximum allowed value
< sipa>
morcos: yes!
< luke-jr>
sipa: we cannot stop segwit for better or worse
< gmaxwell>
Asking miners to produce blocks smaller than the network technically allows is a failed policy, in nay case, as we've seen. The default has been 750k-- but go look at the chain, no 750k blocks to be seen. :P
< luke-jr>
and hopefully in some monthhs larger blocks WILL vbe fine
< luke-jr>
sipa: prqactice is tbd
< sipa>
luke-jr: 'fine' is not a boolean
< luke-jr>
i need to board…
< wumpus>
gmaxwell: on the other hand, miners not dumbly using the default settings is great, it's exactly what should be happening
< gmaxwell>
luke-jr: well nothing special will happen because you aren't here. :P go board, we can talk later.
< morcos>
i think it would be fine to include an explanation of the difference in the release notes without making a judgement about whether or not you should want to have blocks with size smaller than what might be created if you only limited by weight
< luke-jr>
bbl
< jtimon>
the way I see it, the default is stupid to kind of force miners not to use the default (and to avoid being changing some defaults all the time)
< morcos>
if we don't recommend using size but explain why the option exists, then i also see no reason to cache size, if some people want to use it, its ok if its calc'ed on the fly (as current code does)_
< jtimon>
so the issue is that weight has a bigger default than size had?
< wumpus>
jtimon: yes, I thought that was the point
< wumpus>
jtimon: otherwise it's back to 'devs decide the values'
< btcdrak>
sorry I am late. #8412 should be included.
< jtimon>
yeah, and people asking to increase op_return size and shit
< gmaxwell>
IMO we should just make the defaults the maximums. Doing otherwise is thereatics, we know that users will immediately set them to the largest allowed values. And the few that wouldn't will happily manually reduce them.
< jtimon>
re #8412 should I open the PR to 0.13 then?
< gmaxwell>
this argument specifically applies to the 'max weight/size' things because we have expirence there with how they will be set in practice.
< sipa>
jtimon: 8412 will backport trivially, no need for backport PR
< gmaxwell>
(also, FWIW, the income difference between 750k size and maximum is non-trivial).
< jtimon>
no strong opinion, it seems natural to take the oportunity of moving from size to weight to change the default to something more used, but I will complain when somebody asks to change it later
< wumpus>
I really don't like to go back to discussing what defaults to use, e.g. we've had tons of pull requests to change the default block size and we've always used the reasoning explained by jtimon to hold them off
< wumpus>
but indeed, no strong opinion either
< jtimon>
if we want to set a lower default to "force people" not to use defaults there, that's fine with me as well
< sipa>
the current 0.13 code does not change any defaults
< morcos>
wumpus: i suppose the confusion is that now that we have 2 options, its totally non-intuitive how to se tthem given they have non max defaults
< jtimon>
sipa: then what's the problem again?
< sipa>
and the release notes explain the difference, and that only using weight is faster
< wumpus>
yes, what is the problem then?
< sipa>
i think the code and notes in 0.13 are fine
< gmaxwell>
The notes don't seem to inform me how to set it to the maximum.
< gmaxwell>
which is what 99% of the users are going to want to do, as it's what they currently do.
< sipa>
ok, let's fix that in the --help message?
< gmaxwell>
They won't read that. They'll read the release notes with greater odds than --help.
< wumpus>
if it's what they currently do ,then why would the default even matter? they override it already.
< gmaxwell>
(otherwise they wouldn't know of the maxweight option at all)
< sipa>
i assume they'll read neither
< sdaftuar>
(Note that for mainnet, in this release, the equivalent parameter for -blockmaxweight would be four times the desired -blockmaxsize. See BIP 141 for additional details.)
< morcos>
you need to know how to set the limits to max, and also not have the code to unnecessary extra serialization
< gmaxwell>
The default doesn't matter, except it forces people to override, which they'll likely get wrong now that it's been made more complicated.
< wumpus>
in any case the help message and release notes should be clear, if they are missing information that needs to be added
< gmaxwell>
E.g. you'll read that release note and then change your maxsize to maxweight, and then be surprised when their blocks are 250k. :P
< achow101>
why not just set it to the max? I can't see any reason why users would complain about that given that that is what they are going to do anyways
< gmaxwell>
achow101: that was the argument I was making before. wumpus correctly notes that going and debating defaults again isn't great.
< jtimon>
gmaxwell: ok, so you want to just clarify how to select the max block size in the --help because now it's more complicated than before and add something to the release notes?
< gmaxwell>
In particular luke will go on a public attack campaign, and some are trying to avoid it.
< wumpus>
because we should not be in the business of determining what values miners should use, we've always tried to avoid that, for good reason
< gmaxwell>
(because he sees a kind of principled position arising from setting a more correct default which no one uses)
< gmaxwell>
wumpus: not at question of should, it's a question of 'does', and forcing people to set settings which are now somewhat complex due to the new option is a bit unfortunate.
< wumpus>
if you want to inform people how to set the maximum, then adding how to do so in the release notes makes sense
< gmaxwell>
yes, I can make a PR that explains how to do that.
< wumpus>
gmaxwell: we've been there; this really feels like deja vu
< wumpus>
so to be clear: the current 0.13 defaults will cause miners to do something else than create 750kb blocks, as was the case for 0.12?
< gmaxwell>
No, the defaults are functionally equal to 0.12's.
< sdaftuar>
wumpus: no, that is not the case.
< wumpus>
ok, good then
< morcos>
i think the release notes are accurate and relatively comprehensive enough right now. unfortunately, i think the logic that exists is complicated enough that miners will make mistakes and either create smaller blocks than they meant to or slow down their mining computation
< gmaxwell>
the default maxweight is 3 million, which, with no witness txn is exactly equal to 750k maxsize.
< jtimon>
I really don't understand luke-jr's issue if the defaults are untouched
< morcos>
i'm not sure what the solution to that is other than explaining exactly what you should do if you want to mine max size blocks, which might be seen as a recommendation
< wumpus>
explaining in the release notes and documentation is exactly the right thing to do
< gmaxwell>
jtimon: because a maxweight of 3m allows for a 3mb block in future versions with segwit activated and he doesn't want miners being instructed to use maxweight.
< instagibbs>
jtimon, theoretical size max would be 3MB I think
< wumpus>
so people understand what is being done
< wumpus>
instead of saying 'this is too complicated for you, just follow the defaults'
< gmaxwell>
wumpus: 'just follow the defaults' was never my position.
< jtimon>
gmaxwell: why do miners need to be instructed about maxweight before activation? (not that I'm against informing people beforehand)
< gmaxwell>
Rather, we have defaults that match AFAICT what _no one_ wants, meaning that there is no one who can go "oh what the defaults want is already what I want, so I don't have to screw with anything."
< wumpus>
gmaxwell: I know, but that's what would effectively happen if the defaults are just the maximum which everyone uses
< morcos>
gmaxwell: exactly
< wumpus>
gmaxwell: people will start using the defaults, and be recommended using the defaults, which makes us in charge of determining the values. Next release there will be 20 pulls again of people that want to change the default to their favourite value, to lead others into using it
< morcos>
jtimon: they need to be informed because we kept blockmaxsize as an option, but it is slower to calculate (by unknown amount)
< gmaxwell>
jtimon: because he believes its establishing a bad set of practices which will hold over. I think he's right that it will hold over.. but only slightly, people are also going to immediately crank to maximum. I think luke realizes this too, but is adopting a principled position of not supporting something he thinks is bad even if it is inevitable.
< jtimon>
ok, so imagining what most are doing and assuming they don't touch their configuration for now and don't do anything about weigh (ie use the default), everything will be fine, right?
< gmaxwell>
And he think it is bad until compactblock deployment has proven that miners producing larger blocks is okay.
< jtimon>
morcos: I see
< wumpus>
hence, I really think defaults discussion is a waste of time, it just creates more of the same
< gmaxwell>
instead we get worse discussions because we have setting which actively trip up users.
< sipa>
i think we can just leave defaults as they are now
< gmaxwell>
Sorry, wumpus, the software doesn't exist to save us from discussion, it exists to serve its users.
< wumpus>
that's what we've learned from previous messing around with defaults, I don't understand why you've certainly changed your mind about this
< gmaxwell>
I agree that we can leave them.
< instagibbs>
ack for leaving as-is
< jtimon>
mhmm, if they were creating bigger blocks, they will keep doing so, and if they were using the default, they will keep doing so
< gmaxwell>
But we need instructions for setting them to maximum, because as is people will screw it up and produce 250k blocks (and if you care about complaining: they'll accuse us of being sneaky)
< wumpus>
gmaxwell: where did I claim that the software exists to save us from discussion!??
< wumpus>
gmaxwell: wtf is this
< jtimon>
if they move to weigh because it's cheaper but still use the default, the size is the same
< morcos>
I think the lesson to be learned here is we make the code too complicated by trying to be all things to all people. For policy (not consensus) related decisions, i don't think we need to be able to support every possible thing. We waste so much time trying to shoehorn in things like priority and maxsize, while simultaneously trying to make the code more efficient
< sipa>
agree
< gmaxwell>
wumpus: I'm sorry, thats what I'm reading out of your position on the defaults. I feel like you're basically arguing that we should have setting we know are bad and don't match usage, because it allows to refuse to discuss the defaults.
< Eliel_>
Well, you could always just make the mining part refuse to function unless the user manually sets the required config values :P
< gmaxwell>
I think thats an argument that we should make the software objectively worse in order to avoid debates over what is better.
< gmaxwell>
I apologize for reading so much into it.
< Eliel_>
as in, no dfaults
< jtimon>
anyway, it seems this is mostly an issue for luke-jr, so I don't see the point in keeping discussing it while he is on a plane
< achow101>
Eliel_: it's not very user friendly though
< sipa>
how about we leave things as is, and in the future (whenever that may be) we find a way to simplify the options (back to one argument, for example)
< sdaftuar>
ACK simplifying in the future
< morcos>
sipa: i think thats the best we're going to get at this point
< sipa>
that can 1) solve the problem of inadvisable defaults
< gmaxwell>
But we really do need to have instructions on doing what people will do. And when we write those luke will complain because it will be arguable that we are endorsing those settings. Cannot escape the drama. :)
< wumpus>
Eliel_: yes, that has been suggested before
< sipa>
and 2) by changing things at the same time as simplifying the logic, it is not necessarily a recommendation
< achow101>
gmaxwell: just tell him to suck it up and deal with it
< Eliel_>
achow101: that depends... if the error message comes with a link to a good instructional document, it might actually be a pretty good result.
< gmaxwell>
luke only insisted we have the size option because compactblocks is not yet deployed, and if relay network isn't working, orphaning is highly related to size.
< gmaxwell>
It didn't seem like it was worth debating.
< morcos>
gmaxwell: i'd argue we can leave the release notes as is (which luke might still object to) and then just use other channels to explain to people very simply how they can create max blocks if they want. simple. set -blockmaxweight=4000000 and dont' set blockmaxsize at all
< wumpus>
Eliel_: it may be better than setting silly defaults, but there was never agreement to do it
< gmaxwell>
morcos: I can agree with that too.
< jtimon>
gmaxwell: maybe overkill instructions with all the options (keep using the default in a more efficient way (ie not using maxsize) and do what most poeple do) is enough for luke-jr?
< Eliel_>
wumpus: that's pretty much my reasoning behind the suggestion.
< Eliel_>
if you can't set defaults the way most people want them, don't set any.
< morcos>
I'm also fine with that suggestion
< gmaxwell>
luke argued for that for years.
< Eliel_>
and about user friendliness... do you really want to allow people to mine without understanding what they're doing?
< wumpus>
Eliel_: the thing is that people disagree deeply over these setings
< jtimon>
other topics?
< wumpus>
#topic eliminate ISM (NicolasDorier's #8391)
< gmaxwell>
Eliel_: absolutely.
< gmaxwell>
Eliel_: participants in the network should be avoiding judgement, they're not in charge of the system because they participate.. and not because they have some theoretical technical ability to censor varrious behaviors.
< kanzure>
i need to leave. i would like to request that folks send me topic suggestions for my wishlist for the upcoming gathering. not end of the world if it doesn't happen, but having a few thoughts prepared is worthwhile.
< wumpus>
ping jtimon
< jtimon>
this is quite important for other libconsensus refactors, the sooner it is merged the easy it will be to PR other changes, I would really appreciate fast review, test and merge of #8391
< jtimon>
I would say it's the most important libconsensus' PR opened right now
< wumpus>
I think we already agreed removing ISM was a good thing lastm eeting, is there more to discuss about it?
< jtimon>
wumpus: well, I hope not, but that's why I bring it up (apart from asking for review)
< cfields>
ok, I'll approach it as one big change then
< wumpus>
yes it probably makes most sense to do it all as once, it's painful but at least it should be a one time pain then
< NicolasDorier>
question: what you call "refactor windows" what is it ?
< cfields>
ok. iirc there are 1/2 pre-reqs left for boost-specific usage, then basically a search/replace. I'll go back through my notes and do pulls for the pre-reqs.
< wumpus>
NicolasDorier: well the best time for larger changes in 0.14 is now, as there's still some months to go before the release and there is enough time to address problems that come up
< wumpus>
e.g. you wouldn't want to switch the thread library a few days before release
< NicolasDorier>
ok nice to know, as I'm working around libconsensus... will go a bit quicker so
< sipa>
NicolasDorier: we'll need to backport that to 0.13 before segwit
< NicolasDorier>
ok
< wumpus>
NicolasDorier: it's not even merged for master yet, after it is it can be backported
< wumpus>
I'll add a "needs backport" label
< NicolasDorier>
ok no prob
< sipa>
NicolasDorier: thanks for the reminder on that btw
< wumpus>
seems to need some more review too
< NicolasDorier>
jtimon: I'd like to see it go personally, but I don't know the reason why bip34Hash was here in the first place
< sipa>
jtimon: i vaguely remember there were ugly interactions between the cache code that avoids some database operation, bip30 and bip34... but i don't remember the details
< jtimon>
uff, #8259 is ultra disruptive to consensus branch, should review...
< NicolasDorier>
jtimon: yes, I was asking because as part of our work on libconsensus, we'll need to be fixed on that
< jtimon>
yep, you agreed on #8346 first, right?
< sipa>
jtimon: 8259 will need backport to 0.13
< NicolasDorier>
yes
< wumpus>
ok, any final topics?
< jtimon>
sipa: damm, and #8346 cannot be backported because it's a refactor, "refactor winwow interlocking", before a release it's really the worse time to make refactors, but I repeated this many times...
< wumpus>
no, refactors will not be backported, that is too risky
< jtimon>
wumpus: then having the "refactor window" precisely when we're backporting more stuff is a bad idea
< sipa>
it's fine, we'll find a solution for that
< wumpus>
well wait until after the release then jtimon
< wumpus>
hopefully rc2 will be the last
< wumpus>
that'd mean the release can be next week or so
< jtimon>
wumpus: if that still counts as "refactor window" I'm happy, I know when they start but I don't have a clear idea on when they finish
< sipa>
wumpus: the issue is that 8346 won't be backported to 0.13, but 8259 will be, thus 8259 can't be based on that refactor
< sipa>
but i don't think it's a big issue
< wumpus>
sipa: yes, moving the refactor window to after the 0.13.0 release won't help for that
< sipa>
sometimes code needs non-trivial backports
< sipa>
so be it
< wumpus>
yes
< wumpus>
it's still trivial compared to backporting to 0.12
< sipa>
DOING
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Jul 28 20:00:26 2016 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< jtimon>
but in this case means that my alternative refactor to checkinputs will never get reviewed or merged...(even if it's way way older than #8259), so my best option is to rewrite it as nits ot #8259 I guess...
< gmaxwell>
DOOIINNGGG
< jonasschnelli>
sipa: time for your Pokemon walk. :P
< sdaftuar>
cfields: regarding #8096, i don't think i've seen that error since i first reported it.
< jtimon>
haha
< wumpus>
:P
< NicolasDorier>
jtimon: my 8259 in fact date back in march :D
< cfields>
sdaftuar: ok. I tried to investigate, but I couldn't even find a good starting point
< NicolasDorier>
I needed to refactor and reopen the issue several time
< NicolasDorier>
I mean rebase
< sdaftuar>
i recall having no idea what could have happened :) i just fired up a script to run that test until failure, hopefully i'll get something i can debug
< NicolasDorier>
(the first time 8259 was created was on sipa's branch before the merge :p)
< jtimon>
well, my approach is as old as #6061
< cfields>
sdaftuar: only thing I could think of is to add an assert(!empty()) at the beginning so at least we'll be able to tell if it was already empty, if we end up seeing another crash
< wumpus>
sdaftuar: cfields: in any case, the question was whether that should be a blocker; I don't think a rare test failure should qualify as a release blocker unless you strongly suspect it to be a deeper problem
< sdaftuar>
wumpus: i agree that it shouldn't be a blocker
< jtimon>
that was a "please don't do too much stuff at a time" PR for my approach
< cfields>
wumpus: i have no reason to suspect it's a major problem, i just don't like that we can't explain it :)
< wumpus>
sdaftuar: thanks, removing the milestone then
< NicolasDorier>
because it does not run the scripts
< jtimon>
not sure I understand what you just said
< NicolasDorier>
jtimon: in the common piece of code we are touching between #8259 and #8346, I was passing a hashCacheMap as parameter to CheckInput. But it was not even used inside CheckInput because fCheckScript is false.
< jtimon>
oh, yeah, I see, was extending on the comment that it will be shorter for master than for 0.13
< jtimon>
in fact, you can remove the fCheckScript parameter "for free" in the same commit, as now all callers use false. That's for master, not for 0.13
< jtimon>
sorry, now all callers use true
< NicolasDorier>
oh that's cool
< jtimon>
or your life would be easier if you just backported both #8259 and #8346 instead of only one plus the differences of doing it in a different order, and would make people "forwardporting" stuff easier too, but that would be "cheating" by some defintion I am yet to unterstand
< jtimon>
that's ancient coolness that's been ready to pick up for long
< jtimon>
but what I really want to do is always call checkTxInputs way before checkInputs (before noone else calculates txFees in that function [ie ConnectBlock and AcceptToMemoryPool])
< jtimon>
and remove some redundant calculations while helping with encapsulation
< jtimon>
then divide the remaining checkinputs, one multi-thread and one plain simple
< jtimon>
of course there's many ways to do this
< jtimon>
the first version of verifyblock can be single threaded, we don't even have to expose it in libconsensus yet, just having it would be great (we can also expose it in rpc with the storage dependencies I guess, but not sure how useful that would be)
< jtimon>
when we figure out how to expose multi-threading, storage and caches, we can expose verifyblock in libconsensus, but that sounds too long term and that's why I was focusing on maybe exposing verifyheader or verifytx or getflags or whatever was ready first
< jtimon>
but I agree that just complete an internal verifyblock would be great
< jtimon>
and I'm focusing on that now, like you, there's still some stuff I need to move up in my branch because it was about completing verifytx, but it's not needed for completing verifyblock (ie moving things out of contextualcheckblock() )
< jtimon>
btw, for trivial consensus stuff #8413 is also opened
< bsm117532>
ls
< jtimon>
bsm117532:
< jtimon>
drwxrwxr-x 23 jt jt 12288 jul 28 05:27 .
< jtimon>
drwxrwxr-x 12 jt jt 4096 jul 28 05:24 ..
< bsm117532>
[ whereis my brain?
< jtimon>
bsm117532: in your brain and some other parts of your body, but wrong channel again
< bsm117532>
bash: [: missing `]'
< jtimon>
updated #8346
< cfields>
sdaftuar: heh: i just forced a crash in PruneBlockIndexCandidates
< cfields>
not the same crash, but I suspect it's something similar
< GitHub133>
bitcoin/0.13 8360d5b Jorge Timón: libconsensus: Expose a flag for BIP112...
< sipa>
NicolasDorier: i think you misunderstand my suggestion
< NicolasDorier>
mh maybe
< NicolasDorier>
sipa: my point is that lazily calculating the mid state hashes is not worth the added complexity. The only saving is for transaction having no SIGHASH_ALL.
< NicolasDorier>
if a CachedHashes are not read only, then I need to protect read and write with a lock
< sipa>
NicolasDorier: but you solve that by having two CachedHashes
< sipa>
one that's being used inside the computation and modified, and then only copying from the locked map beforehand, and merging back afterwards
< sipa>
your code already does this correctly
< NicolasDorier>
oh I see as part of the TrySet ?
< sipa>
yes
< sipa>
TrySet would merge
< NicolasDorier>
ah ok yes indeed this is simple
< NicolasDorier>
will do that thanks
< NicolasDorier>
I'm a bit too used to everything is a pointer from C# world I forgot I was just using a copy
< NicolasDorier>
for the computation
< sipa>
it also has the advantage of making signature agregation easier later :)
< NicolasDorier>
got it
< sipa>
also, i'd not touch sigcache.h
< sipa>
this is not signature caching
< NicolasDorier>
ok will create a new file for it
< sipa>
thanks
< jtimon>
SIGHASH_ALL is the main.cpp of sighashes
< jtimon>
at some point SIGHASH_ALL should be deprecated, and at some point main.cpp should be non-existent (not probably most users won't agree with this [yet])
< jtimon>
btw, when I talk about "users", the non-existing CPolicy interface is the way I would like to talk with them, with CRandomPolicy on my side to defent some concerns future users may have
< jtimon>
but to be completely honest, when I think about "users" I'm just thinking about me-and-or-potential-future-me's, at least r/btc got that right