< bitcoin-git>
[bitcoin] sipa opened pull request #9868: Abstract out the command line options for block assembly (master...assembleroptions) https://github.com/bitcoin/bitcoin/pull/9868
< rgrant>
if a guard node can protect old mining infrastructure, then nothing that old mining software normally produces makes its blocks invalid. but since segwit scripts are spendable by anyone according to the old rules, some non-segwit blocks must be reliably orphaned, should an attacker get hold of segwit "anyonecanspend" transactions and mine them to different outputs (again, under the old rules). w
< rgrant>
here in the code is this tension resolved?
< sipa>
rgrant: segwit transactions are non-standard to old nodes
< sipa>
so unless the old miners intentionally bypass those checks, they'll never produce segwit-invalid blocks (but they might build on top of a segwit-invalid block that someone else produced)
< rgrant>
yes, this describes an attack. but what makes the attack block invalid?
< sipa>
to whom?
< rgrant>
to segwit-enabled nodes and miners
< sipa>
the segwit consensus rules?
< sipa>
in script/interpreter.h there is a flag SCRIPT_VERIFY_WITNESS
< * rgrant>
follows along
< sipa>
the block validation code passes this flag to the script execution code when it's invoked for a block that has BIP9 activation
< sipa>
and there is conditional code that executes after the normal script execution in that case, which may fail the execution even though the normal execution succeeded
< rgrant>
but the attacker doesn't set bip9 (though bip9 has activated)
< sipa>
yes
< sipa>
that doesn't matter
< sipa>
bip9 activation isn't dependent on the signalling of the block itself
< sipa>
only on the signalling in the chain's history
< rgrant>
right. so segwit has activated, and attacker mines this bad block that rewrites a segwit transaction that otherwise hasn't made it into a block yet. and i'm here in interpreter.cpp VerifyScript, where in my case i believe emptyWitness will be used.
< sipa>
what do you mean by rewrites?
< rgrant>
well, attacker wants to use the "anyonecanspend" interpretation of the segwit tx, under the old rules.
< rgrant>
so they just thorw away the witness
< sipa>
ah
< sipa>
under segwit rules, a segwit output can only be spent by an input that has a witness
< sipa>
the attacker can't control whether the witness flag is set in other nodes
< rgrant>
is that over in CheckInputs?
< sipa>
no, in the script code
< sipa>
in the case an attacker removes the witness from the spending input, you'll still call validation with the witness flag set
< sipa>
and the script execution will fail if it's a witness output that is being spent withoit having a witness
< rgrant>
okay, i get the mechanism. now i'm still trying to hunt down the code.
< rgrant>
(also, does this make segwit outputs viral?)
< rgrant>
wait, the input block needs to have a witness, or the input script?
< * rgrant>
is confused.
< sipa>
the txin spending a witness output has to have a txinwitness
< sipa>
which implies that the block that that txin is in must be a witness block
< rgrant>
thanks. and it can be spent to any address. it's not viral.
< bitcoin-git>
[bitcoin] kallewoof opened pull request #9872: [qa] Multi-chain support in test framework (master...qa-multi-chain-support) https://github.com/bitcoin/bitcoin/pull/9872
< gmaxwell>
luke-jr: are you going to rebase the remaining multiwallet support now that 0.15 has started?
< luke-jr>
gmaxwell: again? I just did yesterday O.o
< gmaxwell>
oh sorry!
< luke-jr>
hmm, I wonder why Travis is whining
< gmaxwell>
s/are you going to rebase/are you going to fix the travis errors/ :P
< luke-jr>
yes ;)
< luke-jr>
assuming it's not yet another false positive anyway
< dcousens>
petertodd: lol, I guess I'll sit this one out then :)
< dcousens>
Did seem difficult to find documentation/material on what you two are referring too though
< sipa>
achow101: it's not perfect w.r.t. the commit, only for the tree
< sipa>
so an attack on the commit's claimed history would still apply
< achow101>
sipa: you mean like if someone faked a commit history with a colliding parent commit hash?
< sipa>
yes, and the same resulting tree
< sipa>
which is certainly much less worrying
< achow101>
ok. but isn't the point to make sure the tree is the right tree and not some colliding tree?
< sipa>
well a commit object in git is both a resulting tree and a history for it
< sipa>
with this method we're only avoiding SHA1 for the tree
< achow101>
right.
< achow101>
another thought: the client version contains part of the commit hash (at least for master). should that be changed to be the sha256 hash instead?
< sipa>
interesting idea
< achow101>
actually that only happens on builds of master, not any of the releases, so not particularly important
< wumpus>
it's part of the commit id - which is there to be able to look up the right revision for troubleshooting. It's not there for security. Please don't replace it with anything else.
< achow101>
wumpus: ok
< dcousens>
wumpus: true, nevermind
< dcousens>
wumpus: my point for the for loop counter was more as a standard, use of `size_t` is the recommended type for when dealing with indexes... while I totally agree this is suitable as `unsigned int`, it was more of a standard
< dcousens>
wumpus: *shrug*, I know, just new code, might as well avoid issues if somehow this gets pulled out later to some generic function blah blah
< dcousens>
not worth discussing any further :)
< wumpus>
this was meant as a quick fix to get an compileissue out of the way
< wumpus>
indeed, no need to drag it out
< wumpus>
whoa, did I still manage to make it fail travis?
< wumpus>
I'm in good company, even "move comment" pulls fail
< dcousens>
ha
< wumpus>
looks like there is another intermittent travis issue: timeouts in make check (probably test_bitcoin related too)
< gmaxwell>
test_bitcoin is really slow...
< wumpus>
well it hangs *before* the "N testcases" line... it's not supposed to spend *any* time there
< wumpus>
still haven't managed to catch the test_bitcoin mutex issue, this may be a different symptom of it
< wumpus>
this is making travis as good as useless
< gmaxwell>
maybe we need to start making PRs with most of test_bitcoin commented out?
< wumpus>
the problem is that it happens during initialization, not during run of the tests. So we'd need to be commenting out global variables and such
< wumpus>
bisecting might work, though that's difficult with unrelabile reprodicibility
< wumpus>
e.g. go way back and find the commit where this started
< wumpus>
the funny thing is that #9851 is passing every single time now that I added debug information
< bitcoin-git>
bitcoin/master 864890a Russell Yanofsky: [qa] Make import-rescan.py watchonly check reliable...
< bitcoin-git>
bitcoin/master c7e57ce Wladimir J. van der Laan: Merge #9839: [qa] Make import-rescan.py watchonly check reliable...
< wumpus>
if it's really an initialization order fiasco, the root cause for sometimes failing could be randomization in e.g. the linker or compiler with regard to what gets put where
< wumpus>
cfields: is it possible to download a PR's cache remotely?
< bitcoin-git>
bitcoin/0.14 eddaa6b Russell Yanofsky: [qa] Make import-rescan.py watchonly check reliable...
< wumpus>
hm no, never mind, I don't think that's going to help at all either...
< bitcoin-git>
[bitcoin] laanwj opened pull request #9873: [do not merge] upload test_bitcoin executable for #9825 (master...2017_02_travis_upload) https://github.com/bitcoin/bitcoin/pull/9873
< wumpus>
awesome, I started debugging #9825 again and all pulls start passing! Start observing and the bug goes away, stop looking and it comes back. The unobserved state is a fog of probabilities, a window of and for error. *wheee*
< wumpus>
Victorsueca: either that or we need an exorcist
< wumpus>
luke-jr: this doesn't seem the typical issue "test/rpc_tests.cpp(73): error: in "rpc_tests/rpc_rawparams": unexpected exception thrown by CallRPC(std::string("signrawtransaction ")+rawtx)"
< luke-jr>
I can't reproduce it :/
< Victorsueca>
maybe the code is ok and the problem is at travis?
< wumpus>
luke-jr: restarting it...
< wumpus>
Victorsueca: it's possible, though that doesn't explain why it always fails in the same (or similar) place
< Victorsueca>
so either the code is bad or travis is using some non-standard compiler that throws shit at the fan when it hits that part
< luke-jr>
hmm, I wonder..
< wumpus>
it should be using stock trusty gcc - but yes it seems some non-determinism/randomization in the compiler is causing a bad test_bitcoin sometimes
< luke-jr>
is wallet_tests/rescan really acceptable? it's assigning pwalletMain for a test to a stack variable, and leaving it there?
< wumpus>
luke-jr: it'd be cleaner to reset it to NULL at the end of the test. Hopefully, though, that whole nonsense can go away when there's a better way to pass in the wallet via RPC context
< luke-jr>
I wonder if that's the cause - signrawtransaction can use the wallet when available
< wumpus>
but yes any test after that that assumes pwalletMain is set to something will crash or do bad things
< luke-jr>
I just can't reproduce to confirm
< wumpus>
yes, thinking about it - WalletTestingSetup sets up pwalletMain. Setting it to NULL would be bad too, it needs to be restored to its original value?
< wumpus>
yes, you might be on to something
< wumpus>
the next test using the walletMain after that will corrupt
< wumpus>
+the stack
< bitcoin-git>
[bitcoin] laanwj opened pull request #9875: tests: Fix dangling pwalletMain pointer in wallet tests (master...2017_02_wallet_test_dangle) https://github.com/bitcoin/bitcoin/pull/9875
< Chris_Stewart_5>
would not be serialized as the entire tree when sending it over the network, correct? It would prune the 'rightmost' branch 'f' right?
< Chris_Stewart_5>
and just provide the hash for 'f'
< sipa>
that's about how the merkle root hash is computed
< sipa>
not what is in it
< sipa>
it's illegal for a transaction to be repeated inside a block
< sipa>
yet, if there is at any level of the tree an odd number of entries, we duplicate the last one _for the purpose of computing the root_
< sipa>
that case is handled in the else branch of the MerkleBlock code you linked to
< Chris_Stewart_5>
Hmm ok, so if we have a block with 3 txs in it, and we wanted to send a merkle block to a spv node to prove a tx occurred in that block, we wouldn't have to duplicate that 3rd txid to recompute the hash of the parent node? and thus prove to the SPV node the tx occurred?
< sipa>
that if selects whether we're in the normal case, or the 'last entry of an odd-sized level' case
< sipa>
only in the first one we require the two lower level hashes to be different
< sipa>
in the second case there still is only one hash, but we duplicate it for the purpose of computing the parent level
< Chris_Stewart_5>
yeah, that is what i was missing. Thanks.
< luke-jr>
wumpus: weird, re-pushing with a new commit-id (no code changes) passes Travis now
< petertodd>
dcousens: don't feel bad, it's a very confusing issue; and my apologies if my answers were a bit terse! was trying to get something else done at the same time
< petertodd>
dcousens, sipa: my best advice is to get a copy of the opentimestamps client and run the git integration code with the verbose switch on to see exactly what's happening under the hood
< sipa>
petertodd: i don't think that's useful without reviewing that code myself :)
< petertodd>
sipa: I just mean, it's an implementation of something that hooks into the low-level git signing code, so it's an easy way to see exactly what's being signed
< sipa>
it seems my gpg key (which was created in 2015) used sha1 to sign the subkeys
< sipa>
horror.
< petertodd>
heh, yeah, I think the defaults were changed pretty recently...
< bitcoin-git>
bitcoin/master 988ce2d Chris Stewart: Adding 'amount' label to tx_valid/tx_invalid.json files
< bitcoin-git>
bitcoin/master 88c2ae3 MarcoFalke: Merge #9350: [Trivial] Adding label for amount inside of tx_valid/tx_invalid.json...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #9350: [Trivial] Adding label for amount inside of tx_valid/tx_invalid.json (master...master) https://github.com/bitcoin/bitcoin/pull/9350
< dcousens>
petertodd: no problem :), we all got there in the end :)
< jeremyrubin>
Is anyone else kinds surprised that github didn't have a ready to go sha256 (or whatever) version of git?
< sipa>
what does github have to do with that?
< sipa>
it's perhaps surprising that git itself never prioritized upgrading to a better hash function
< jeremyrubin>
They have a lot of money on git
< jeremyrubin>
it's like a bitcoin company not investing in any core dev ;)
< sipa>
no
< sipa>
it's like a bitcoin company not developing their own altcoin
< dcousens>
haha
< jeremyrubin>
Example:
< jeremyrubin>
github could run an internal git chain
< jeremyrubin>
using upgraded hashes
< dcousens>
jeremyrubin: how do you know they don't?
< sipa>
jeremyrubin: you're missing my point
< dcousens>
GitHub is about compatibility with git
< jeremyrubin>
could test it out pushing the broken pdfs :p
< sipa>
either changing git's hash function is easy, and a fork or branch with support for it would be sponsored, and be available, and they'd use it
< sipa>
OR switching the hash function is hard, so nobody has a working version
< sipa>
suggesting that github would do so without compatibility with git itself seems ridiculous
< jeremyrubin>
idk -- I would expect a large fraction of github users could pretty quickly migrate to "git2.0" if github sent them an email detailing the changes
< jeremyrubin>
Maybe I don't use very many tools on top of git
< sipa>
you're literally saying here that github fork their own version of git
< bsm117532>
jeremyrubin: I've been very surprised there doesn't seem to exist a sha2 version of git. I've thought about writing one myself. Especially integrating opentimestamps and commit signing...
< jeremyrubin>
sipa:yeah, I think they should. It's not bitcoin :p
< sipa>
jeremyrubin: I think *git* should support sha2
< luke-jr>
jeremyrubin: to rephrase: you're saying GitHub should stop supporting git ;)
< jeremyrubin>
yeah, but it seems Linus has his head... somewhere
< cfields>
Linus' mail said it's in the works.. does it not exist somewhere?
< jeremyrubin>
They can support both?
< jeremyrubin>
No reason to not have git and gitsha2 available simultaneously.
< sipa>
i think i'd stop using github if they did that
< dcousens>
jeremyrubin: I suppose it depends on how its done... maybe its a soft-fork haha
< sipa>
it would feel like an embrace-and-extend
< sipa>
if they truly felt sha2 is important, and willing to put money on that, they could subsidize or contribute towards sha2 support in git itself... not suddenly switch to an incompatible fork themselves