Bitcoin 0.10 has been released - what should we do? #286
Comments
|
How does the Omnicore process the omni transactions now. Are they triggered during the normal bitcoin tx processing? If so how about adding a middle man step. High level thought process: |
|
Every time a new block is connected, it is forwarded to the meta layer at the moment. For 0.10 instead of forwarding every new block, a block should be forwarded, when it's the very next to the last processed one, and the forwarding should continue, until no next block is available. But there are also "higher level" signals to subscribe to and it's not necessarily required to hook add handlers into #263 worked, back in December, although I tested it basically only fully synchronized with new blocks added to the top. The biggest hurdle is probably UI related, because it has changed significantly:
|
|
From my perspective (someone developing software to the RPC API) the 0.10.0 release is very desirable. Several of the improvements (faster blockchain downloading, RPC "warm-up" mode) address pain points that come up in testing/integrating. There is also a fix for building on OS X 10.9. And I almost certainly will update Bitcoin-Qt on my Mac. |
|
Sorry, for clarification I mean strategically. So yes absolutely we want to move to 0.10, but that will take time (an unknown quantity at this point). Moreso things like:
The technical stuff is still important, but moreso end user facing issues such as:
The actual questions surrounding a technical migration would be addressed as we actually worked towards OmniCore 0.0.10. I'm moreso concerned with the here and now - our ability to a) deliver this platform in a working fashion if we release now and b) our ability to support the platform in the context of the incompatibilities between the 9 and 10 branches of Bitcoin. Hope that makes more sense :) |
|
I guess I'm saying I support switching to 0.10.0 before making any new releases -- including the UI. At the very least we should take the time to quantify the effort before deciding. |
|
So the decision was made to go ahead with a 0.0.9.1 based release, along with some code to force a shutdown during startup if we detect a blockchain from 0.10. First question now to all - before I spin up the VM's to test, has anyone actually tried to run OmniCore 0.0.9 against a 0.10 blockchain? @msgilligan you mentioned you thought code was already there to prevent 0.9 bitcoin clients running when a 0.10 blockchain is present - have you tested this by any chance? Thanks all |
|
I haven't tried it, no. I don't have any specific knowledge, just a hunch given what I know of the Bitcoin core team and the fact that this feature was on their roadmap for a long time. The Downgrading section of the 0.10.0 release notes says "It is possible that the data from a completely synchronised 0.10 node may be usable in older versions as-is, but this is not supported and may break as soon as the older version attempts to reindex." |
|
OK great, thanks for the info - that's kind of what I was concerned about - have a bitcoin 0.10 install and run Omnicore which may "break as soon as the older version attempts to reindex" - that kind of indicates to me that an older version will attempt to reindex, rather than cleanly shutting down. I'll have a poke around and see what I can come up with :) Thanks |
|
Just playing devil's advocat here for the sake of being overly protective: the risk is that blocks are stored out of order, because they arrive out of order, and an old client may not make sense of it. I assume that this risk is also given, if the user is just a few blocks behind and catches up.
Reindexing and fetching the last blocks doesn't seem much different. Edit: this was only a guess, but after thinking more about it: blocks are indexed, and a 0.9 client wouldn't necessarily locate blocks by scanning files, but rather looking into the blockdb, so the statement regarding reindexing gains more significance for me now and it is also a bit different than catching up. @zathras-crypto: do you have an idea how to identify it? |
|
Alrighty, testing this out myself ATM. Have taken a snapshot, deployed 0.10 and spun it up. Firstly no idea why it needed to write over a GB of delta when 'verifying blocks' - assuming it's doing more than just verification there. It still appears to be syncing in sequence (days going down one by one, blocks processed going up sequentially about 50 blocks at per update). |
|
OK. I performed the following test:
First thing to confirm, there is absolutely no detection or protection against using the bitcoin 0.10 blockchain with an earlier client (at least bitcoin 0.9.3 (our base) anyway). OmniCore fires up no errors or warnings. |
|
Still nosing around looking for a way to detect if 0.10 has been used on the blockchain, but I did notice that even on 0.10 the blocks seemed to be processed in sequential order. Perhaps this is because peers aren't running 0.10? |
|
You should try |
Not that I want to hijack this decision, but I was curious about the block connection behavior and ended up porting Omni Core without UI to Bitcoin Core 0.10 in the last hours. All regtests passed and at the moment I'm synchronizing on mainnet and dump every handler call. Looks good so far! And it appears that
I'm away for a few hours now, but I'll try to push everything wrapped into expressive commits later. That being said, porting was so much easier than assumed, but only because I prepared most of it earlier, e.g. cleaning up the includes etc., ... :) But key was also to start with 0.10 and add the components one by one, instead the other way around as last time, which came with like 100 merge conflicts. >_< As heads up, those are requirements:
... and my current build also includes some of the other PRs, right now to ensure it also passes the other tests I have. The incompatibility issues can be resolved in ~15-30 lines or so in total, so reviewing the PRs is probably most of the work for Omni Core based on Bitcoin Core 0.10 without UI modifications, but plain Bitcoin Core interface. Edit: to clarfiy: graphical interface. All the RPC interaction work fine. @zathras-crypto: what do you think about porting the QT code? As very rough guess I'd assume the tricky part is to deal with 0.10's new interface, while the code blocks could probably be copied one to one. Will take a look at this later as well, if I find some time.. |
Hehe, that's also Michael's preferred methodology :)
I think the primary issue is that I don't think anyone is comfortable changing our underlying bitcoin version in a minor release. It's GREAT to know this can be progressed in such a straight forward manner, and well done my friend :) but when looking at a risk angle - to play the safe card we need to change our base Bitcoin code in a major release not minor. To be frank, the issue is driven by timing primarily. The powers that be want the UI released yesterday and putting whatever QT work is required aside for a moment, I would still insist on a whole bunch of testing for a new base bitcoin version. I really do appreciate the testing you've done so far, but for me I just don't feel comfortable enough yet to say "in 100% of potential circumstances there is absolutely no possible change to Omni data in bitcoin 0.10" which is needed to do a non-major release. Sorry, not wanting to ramble on - long story short the main driver is to do 3 things:
Once that's done, we could even skip 0.0.9.2 and move straight to a 0.0.10 with bitcoin 0.10.
To be honest I don't think it'll be a major drama, I'll be applying similar logic to you & Michael with the copying files over - though there may need to be some time spent in QT designer adjusting some of the forms (that's always fun! /sarcasm) |
|
I'm honestly having quite a hard time trying to find a way to detect a 0.10 from the blocks or chainstate programatically. So far I can quite easily identify it by checking if the debug.log has certain elements that were introduced with 0.10, but that does not feel like a safe or solid way to do it. Any suggestions welcome :) Thanks |
Oh damn, I hope you're well!
Please don't get this wrong, but given that there was not a single announcement on either one of the mailing lists, the forums or the blog about the last consensus critical release, I'm wondering: which release process? The regtests are by far not complete and relying on them only would be crazy at this point, though it's unfortunally the only route of testing, besides consensus checks, that is currently available. One reason I'd value increasted testing is to replace "feeling it may or may not work" by "knowing this or that definitely works, confirmed by tests". A sort of long way still. As mentioned earlier, I fully support the conservative approach, but I'm happy about the realization it worked out better than expected. :) But I also agree that pushing the UI release should be highest priority!
Actually this is should only be a workaround and not providing the full commit history is a no-go in my opinion, but it should be possible to cheat here: 1) get the desired result, 2) merge from scratch to preserve history, 3) see tons of conflicts, 4) replace files with desired result from earlier, 5) commit. Back to the actual issue: It would certainly help to know what risk there might be, to shutdown as consequence, if things go south. I assume your 0.10 datadir is synced now? If so, and if you don't mind, I'd be very curious about what would happen, if it's used in combination of reindexing and Omni Core 0.0.9.1. The two relevant questions in this context:
I'm not really sure how the reorg handling and persistence layer of Omni Core comes into play here, but it's guaranteed that no new transaction reaches the meta transaction processing, if the block it belongs to isn't the a very next to the earlier one. See the assertion at the beginning: // Connect a new block to chainActive.
bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew) {
assert(pindexNew->pprev == chainActive.Tip()); //<! THIS ONE HERE
(void) mastercore_handler_block_begin(GetHeight(), pindexNew);A few lines later is the actual transaction handler call, which won't be reached by out of order blocks: BOOST_FOREACH(const CTransaction &tx, block.vtx) {
SyncWithWallets(tx.GetHash(), tx, &block);
if (0 == mastercore_handler_tx(tx, GetHeight(), tx_idx++, pindexNew )) ++countMP;
}
(void) mastercore_handler_block_end(GetHeight(), pindexNew, countMP); |
Thanks dude :) Just got results for CT - taking a glass half full attitude - still don't know what's wrong but given what the Doc suspected I actually shed a tear (call me a p*ssy if you will hehe) when I rec'd the all-clear for that particular diagnosis this morning - sadly more tests now to figure it out but it's pretty darn relieving when the Doc suspects bad things and the tests come back clear :)
If there was a way to +1 that comment enough that people would take notice, I would! Honestly mate, I've been raising the issue of a lack of process for release for some time now it just doesn't seem to get much traction :( though I honestly consider it one of the most important pieces to a successful progression on this project.
Yeah agreed - trying to play the conservative approach but it's really not that easy to identify whether blocks have been written out of order. I do have a methodology but it's loooooooooong-winded and I don't want to use it (it involves looping CBlockIndex for each block and looking up the blk???.dat file it's stored in along with it's byte position and undo locations in the rev???.dat files and then ensuring they're sequential from the previous block). Also scraping debug.log works but isn't what I'd call solid (eg what happens if user deletes that file). I was actually surprised as I thought there would at least be some change to CBlockFileInfo regarding the new way of doing things but it seems not sadly.
I don't believe it would, but then that's not backed up by anything other than assumption on how certain things work.
Well I was actually chatting with @m21 about that this morning, he made a suggestion of keeping our own eye on things in our own handlers so we could thus make sure to abort the node if for some reason the block handler was called on an out of order block. I don't think it solves the issue because the issue is more one of older clients being able to get at out of order blocks rather than processing out of order (as you note, connecttip should always be called sequentially otherwise core would be processing bitcoin transactions for which the inputs didn't exist yet) but I still think it would be a nice sanity check to drop in given the ease of doing so. I continue to believe the path of least resistance is to block 0.10 users from using OmniCore 0.0.9.1 and get it released, and then just put all our effort into a OmniCore 0.0.10 branch that uses the new bitcoin 0.10 base and do the requisite analysis and testing while we're not being pressured to release. Just getting there seems to be more of a PITA than I first thought hehe. Thanks |
|
Figured I'd commit what I'm working on at the mo so you can take a look mate - basically now I'm avoiding loading the blocks from disk and purely using the index it's a lot faster - would still like a better way to do it but this is what I'm poking around with and testing out atm zathras-crypto@325a3b9 Thanks |
|
FYI: |
|
A few 0.10.0 goodies related to RegTest mode that I just discovered: bitcoin#5275 (RegTest performance improvements) |
|
Ah, great progress! First off, again, @zathras-crypto: hope you're well! I tested this approach and it works! Neat idea of checking it this way and I can confirm out of order storage of my 0.10 files. I'm very curious what happens, if I resync 0.9 now .... hehe. Just wondering about all the vectors and maps? Check this out: https://gist.github.com/dexX7/807cb7a0e26383d616d0 While thinking about this, and I really have no idea if it's feasible: would it be possible to copy the reindex logic of 0.10 to support out of order storage, if it turns out this is the primary issue? @msgilligan: Yeah, the block generation is notably faster. :) |
|
Using 0.9 as it is works well, even when reparsing all Omni transactions, but reindexing with 0.9 causes a segfault. |
Thanks mate :)
Both of those gists look like interesting methods to simplify what I have mate, nice - let me take a poke around them :) FYI that code is a reimplementation of PrintBlockTree which is where the maps/vectors come from - probably not necessary (hadn't optimized at all, only got as far as getting something that 'works'). I had the idea on the 'how', and wrote my own code for it but it was slooooow as anything because I was loading each block from disk to analyze. So I went hunting for a method I could use to just do it via the blockindex. Some grepping later I found that the bitcoin devs had the PrintBlockTree function which looked like it would serve as an excellent starting point for this, so I cloned the function and modified it to do what we wanted.
How would you rate the value of this? In my head I had a kind of "get 0.0.9.1 out and then shift all our resources into a 0.10 based branch", so I hadn't really envisioned putting too many more resources behind a 0.9 based branch after 0.0.9.1.
That's quite interesting actually. So an end user may get away with it (if we didn't have the abort in place) as long as they already had a txindex, but a user who uses a fresh bitcoin 0.10 and then fires up Omni (which enables txindex and thus needs a reindex) would see a segfault? |
|
Impressive. The following commit works out of the box with 0.9 and resolves the reindex issue. Maybe it's too early to say, but I feel very confident that this can be used for 0.0.9.1 to handle out of order blocks and Bitcoin Core 0.10 files: bitcoin@ad96e7c Edit: and the log entries confirm: |
|
I'm going to fully This is less restrictive than forcing a shutdown, and - based on the assumption that the actual risk is related to reindexing - it should be safe, even if the warning is ignored. As final test, it would be very good to know, if synchronizing (not reindexing) from scratch works as well with this commit. There seem to be five user groups who may use Omni Core:
The handlers are furthermore guarded against out of order blocks, so, at least regarding the state of Omni/Master, the risk potential seems low. But I'm still not entirely sure, if there are other side-effects. |
|
FYI from a discussion I had with @m21 my view was essentially:
That's me being risk-averse (probably my background in infra mgmt :P). However when considered in the context of:
So I guess it comes down to the question of if the bitcoin devs don't want to support using 0.9.x clients on a 0.10 blockchain, do we? I do concur with your analysis above though and the note specifically relates to reindexing breaking, so if we fix that the risk does appear low. EDIT "completely synchronised" - what about a node that's not completely synchronised? Ie user starts with a 0.10 node, synchronizes half of it (so has a complete set of headers completed via getheaders but only half the blocks via getdata) and then fires up 0.0.9.1 |
I notice that commit is based on top of the headers first PR - is it safe to cherry pick it without the underlying PR? |
|
Yes, cherry-picking should in work, but it created conflicts for me, but only limited to the changes itself. Otherwise copy and replace by hand. Regarding 0.9 in general: it's notable that rather sooner a softfork related to BIP 66 kicks in, see: https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki#deployment This doesn't make 0.9 unusable, but I tested it earlier on regtest and as result a yellow warning in the UI is shown: You may spot entries such as the following in the debug.log: |
|
Either way, I think pushing the release as soon as possible should be a goal, and as next step the migration to 0.10, which appears to be rock solid (appears.. hehe) as server build, in terms of mainnet consensus and all tests that are available. |
|
Well sadly GitHub process isn't my strongest suit mate, but I had discussed the process with Michael and agreed it essentially as:
TL:DR; I thought you basically had it with
Basically as long as we have a way to track the history of what's been changed I'm comfortable :) |
|
So just to clarify: the Bitcoin Core 0.10 based branch is basically ready: That's the one you took a look at and it's last state is also the one I'd build upon. It contains the full Bitcoin Core commit history and the commits I made, starting with the "copy mastercore*" files commit, followed by the upgrade patches. What I intended was to restore the whole commit history of the Omni/Master Core related commits as well, which I consider as nice to have. Alternatively, I could edit the commit message of the "copy commit" and add a link to the last commit of which those files were based on. This would be doable within a few minutes, but that comes at the price of no Omni/Master Core related commit history prior this point. As next step, one of the owners, namely @CraigSellars or @achamely, would have to copy/clone/mirror the current |
|
Right right that makes much more sense, thanks for the clarification :) |
|
Fine, so what do you suggest to do now? :) |
Adam, could you do the honours mate? |
|
In case I wasn't clear guys this is ready to go - I've checked the mastercore_* files were unchanged and the subsequent commits were clean - it's literally just waiting on the repository move @CraigSellars @achamely :) |
|
Just a little chase :) can we get this repo move actioned? (I'm working locally atm) |
|
Well yeah, would be great. :) Please publish/push nevertheless. :) |
|
Heh, not so sure I should publish what I have right now lol it's a bit of a mess :P (I'm in the middle of redoing the UI for 0.0.10)... As a side note, how would I do that on github (when I'm ready to push)? Right now I'm working on a new branch I checked out |
|
Ah I see. :) I usually start with a new branch, based on the current tip. Let's assume To switch to that branch: Refresh your local copy and pull updates, if there are any: You probably already have done something similar, but with my branch. From here: Add your remote: Create a new branch: Do some work, add some commits. And finally push: To remove a branch locally:
Hope this helps. :) |
|
Just catching up on this thread now. We have to options, I can either Move the existing repository from mastercoin-msc to omnilayer and rename it to omnicore |
|
My preference would be to clone rather than move :) |
|
Great to hear. |
|
Sean mentioned in the meeting today that it might be better to not fork from Bitcoin and just do a fresh push to the new repo from a local clone, as this helps with certain stats? Any feedback on that @dexX7? Otherwise it'll be a new fork of Bitcoin, not a move of the old repo. Thanks :) |
|
After discussing in the meeting we are going with a fresh fork of bitcoin to preserve upstream visibility and allow changes to be pulled/pushed? in future. its up on https://github.com/OmniLayer/omnicore now. |
Thanks. This would have been the point I'd raise as well. |
|
@achamely: given that Bitcoin Core 0.10 works very well with Travis and the base file already exists - even though it fails with Omni Core due to the wallet and txindex requirement at the very moment - I suggest to enable Travis for |
|
Ah, and please enable "issues". :) |
|
Issues enabled (as well as wikis) |
|
@dexx i know @msgilligan has already got a jenkins ci environment setup. How is travis different/better? |
|
@achamely: It's not better, but what I really miss are automated tests of pull requests. Doing consensus tests and alike should still be done via the CI server - it's more about using what is already given, and leveraging the test infrastructure that comes with Bitcoin Core. In particular this includes testing of ARM, Linux, Windows and OS X builds with different configurations (example). Depending on whether this is an option, I'd take a look at how the "txindex" and "with wallet only" build requirements could be addressed (during testing), which would currently result in test failures. |
|
Jenkins supports automated tests of pull requests, but there are security and configuration issues in setting it up and I haven't had time to resolve them. |
|
@msgilligan: sorry to be unclear, this is acutally the point: it is costly, in terms of setup time, risk exposure and additional load on the server, which is better be used for the core operation - deep testing of Omni Core. It should probably be aimed for to provide a similar set of features and tests via the CI environment, but until then I actually see no reason not to use Travis as complementary. OmniLayer#4 tackles the issues related to txindex and wallet requirements, which I mentioned earlier. |
|
I think we're in agreement. Absolutely use Travis as a complementary solution. However at some point if I find an easy way to combine Docker and Jenkins -- or need to do it for another reason -- then we can revisit this. |
|
Ah thanks @msgilligan. Ideally we would run both test suits, to ensure all builds are fine and pass the Omni Core related tests. I'm running Travis on my bitcoin/omni repo for a while and yesterday noticed it's already pretty close to the maximum build time of 50 minutes, so I guess there is no way to squeeze it in as well. If it turns out those 50 minutes are not sufficient, I even guess the number of tests would have to be reduced. Unfortunally Travis doesn't allow caching for "non container builds", but using "container builds" is currently not possible, because sudo (and thus apt-get install) is not allowed there.
|
|
@msgilligan: But I also played around a little with cross compiling on my Ubuntu VM, and this part was actually easier than anticipated. In another fresh VM I then setup Jenkins and created a job with multi-configuration build matrix. Turned out this works different than expected, but I think I got it now. The key was to have only one build matrix parameter, named "ARM" "bitcoind" "64-bit_qt" "32-bit_dash" "Cross-Mac" "Win64" "Win32"As far as I can see it's not very different to standard jobs, except the build matrix:
For the actual build step, I converted the Travis configuration into a shell script: There were a few issues so far:
I'm currently running this configuration and not all builds with spock tests finished, but the Windows builds (with Bitcoin Core tests) appear to work well, and the spock tests I saw so far, too. :) What do you think about it? Re: pull requests and status updated, which I posted earlier, but just in case: Edit: even though I'm testing building via a build matrix, I assume it's more convenient in terms of test output handling, when using more than one job. @achamely: |
|
Hi @dexX7 I'm 100% focused on OmniJ for the next couple of days. I'd love to see cross-compiling on Jenkins, though. |
|
@achamely @CraigSellars (as organization admins): Please enable Travis CI for automated testing, including pull requests for OmniLayer/omnicore. While this doesn't cover the RPC tests via bitcoin-spock, it does test six different build plattforms, a huge number of Bitcoin Core tests, and of course the unit tests. Given the increased number of unit tests currently added, this becomes more and more relevant. It takes 5 minutes and simply consists of a GitHub authentification. See: #269 (comment) |
|
We have now a Bitcoin Core 0.10 based branch over here: This can be closed in my opinion. |
Hi all,
I'd like to get a discussion going with as much participation as possible. The issue is simply that Bitcoin 0.10 has been released, and this is not compatible with OmniCore 0.0.9.1 (due to headers first).
We're gearing up to release the UI, however we need a strategy to handle the issue. We can't have our first major end user release being a failure because we haven't planned for this appropriately.
Thoughts please?
Thanks
Z
The text was updated successfully, but these errors were encountered: