Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This fixes the PoS rewind issues. #12

Closed
wants to merge 2 commits into from

Conversation

breakcrypto
Copy link

SetBestChain is broken (see: https://bitcointalk.org/index.php?topic=331394.msg28128479#msg28128479 and later comments for full details). TL;DR: paccoin is subject to rewind attacks, double spend, and lost work -- this needs to be fixed ASAP.

Bump the version because this is a 'breaking' change and old clients are subject to PoS rewind attacks.
@wmcorless
Copy link
Owner

changing the protocol version will cause the old clients to no longer sync with the new clients. this will definitely result in a fork.

@breakcrypto
Copy link
Author

It should fork, it's broken.

@wmcorless
Copy link
Owner

have you tested it?

@breakcrypto
Copy link
Author

On my own fork, yes. It kept the chain stable. The net result is that when a bad PoS block which would rollback the chain comes in, the client ignores it and keep the chain moving forward and the PoS block that would revert is ignored and the proper chain stays set as best.

Without considering height, the PoS blocks are getting too much 'trust' value which is wiping out newer PoS and any new PoW which, of course, is a HUGE issue.

@breakcrypto
Copy link
Author

I should have added, it's easy to test out. Sync up your client with the current chain, grep you debug.log for SetBestChain -- try one with the patch and one without. You'll see the one without become unstable within a few blocks at the point, the one with the patch will grow properly with PoW and PoS blocks in order -- like they should.

@wmcorless
Copy link
Owner

does this fix allow users to mine without orphans?

@breakcrypto
Copy link
Author

I never hit any orphans while mining (and I think I'm the majority of the chain lately since I was testing this for the last few days). I mean, if you're mining while the chain updates to a new block, you'll discard the block like you should, but that's expected behavior.

Right now, blocks are orphaning at a near constant rate. There's a ton of TX sitting at 0 confirms because of this PoS issue which really should be resurrected but I've been too busy with work to dig into the fix for that -- sorry.

@wmcorless
Copy link
Owner

I'll consider the patch without the version change for now. changing the version requires getting the exchanges to upgrade which is difficult since they are slow at making changes, and it will definitely affect the users.

@breakcrypto
Copy link
Author

The change will 'hard' fork even though it'll 'soft' fork in reality. It creates two chains. Those with PoS rewind attacks and those with a functioning change. I get the exchange issue but unless a critical mass of users update to the latest client, they'll actually be in a worse position until their chain reaches a higher 'trust' value.

If you want to soft fork it, lower the trust value that PoS blocks are adding to the chain. Right now, a badly crafted PoS block easily consumes hours of mining. Again, however, that's still going to soft fork.

I've gone through the code several times and I can't think of a way to fix this without a hard fork on the version. Any soft fork is just going to result in people with the latest chain mining to the alternate chain and they will, hands down, lose unless a massive wallet holder is staking to that chain which will up the trust level higher than the other broken chain.

@breakcrypto
Copy link
Author

http://paccoinexplorer.com/address/ApJdYmRR6yuQHqWWxAxG5zS4ZFCkeD6w9S

This wallet for example is sending bad timestamps and will eat the chain...every time. Of course, if whoever had the wallet was on the softfork, it would hold and beat out just about anything else because of the trust score bump.

@wmcorless
Copy link
Owner

You mentioned you mined this coin early on, do you remember when it was doing those horrible roll backs?

@breakcrypto
Copy link
Author

breakcrypto commented Jan 15, 2018

Yes, I mined it shortly after launch or on launch. Not for a very long time, I was testing a new ASIC to be honest, but the stake has grown over the last years which is why I still have the wallet, lol.

Anyway, I suspect it's been broken since the new trust code was added. I think it's only noticeable when the PoW activity is high since anyone mining PoS properly and in-sync with the client would help the chain gain trust but anyone exploiting it on purpose (or just having an out-of-sync client) would revert the chain. The reason I say PoW activity high is because PoW blocks are getting way, way less trust than PoS. So you can mine a ton of PoW and then a bad PoS block comes in and poof all those blocks disappear. Right now, it looks like about 60 blocks at a time, but I've definitely lost over 10 hours of mining in the last few days a few times which is definitely concerning.

I started mining again about 4 days ago and noticed this happening. I started digging then but it took me awhile to figure out why I was "losing" my mined blocks.

Ex. If your client's height is back 100 blocks and you mine a large PoS block, chain reverts. If you're in sync and mine a large block -- it'll make the current chain stronger.

@breakcrypto
Copy link
Author

If you wanted to check when it's forking, it'd be trivial. I haven't written the code to do it, but just scan the block chain for when the timestamp reverts backwards.

It's easy to see on the explorer or in the client, super easy. You can watch the SetBestChain output or just watch the diff revert and the block heads. I modded my client to alert me and it does it at alarming rate.

@wmcorless
Copy link
Owner

I watch the debug log and see it reorganize, I haven't seen more than 3 blocks, at a time though.

@wmcorless
Copy link
Owner

i also get this: "ERROR: mempool transaction missing input"
what causes that do you know?

@breakcrypto
Copy link
Author

  1. It'll do far more than 3 blocks; that I can assure you. I've watch it revert at least 60 at a time and I'm pretty sure one night it consumed over 10 hours but I wasn't recording the logs.

  2. Yes, I was digging into that. From what I was able to suss out, it's a result of a TX occurring in the wallet that was on a forked chain. The result is your wallet has a TX number that is no longer valid on the current chain. I haven't verified that though, so please take that with a grain of salt.

That said, if you make a new wallet and re-run the daemon, it'll run clean, and it'll stay that way until you send a TX that forks -- that's what I'm basing my theory on.

@breakcrypto
Copy link
Author

I think an extra check on the timestamp for PoS blocks would be a help -- not needed with the height fix but scanning the time would definitely add extra protection.

Working on a way to pull the TX that get overwritten. I tried porting the -zaptransaction code from BTC core. I got it in and compiled but it didn't seem to work on the wallet I tested. Rescan isn't picking up the lost TX too since they're still being seen as valid just unconfirmed.

@wmcorless
Copy link
Owner

This code accepts all blocks, whether it should or not. The purpose of orphaning a block is that the network rejects it. This is part of basic consensus building. This fix actually allows unlimited forks to grow, which will result in a broken coin.

@breakcrypto
Copy link
Author

Miners should discard the bad chain once SetBestChain is called and switch over to the highest trust. You could explicitly ORPHAN the block if you wanted.

Right now the chain is forking about every 30 seconds or so. I just watched it reset from a diff of over 4k to 90 due to a PoS reset.

@wmcorless
Copy link
Owner

this change breaks the coin, so will be disregarded

@wmcorless wmcorless closed this Jan 20, 2018
@zahidaliayub
Copy link

What about the POS miner is still in pending of last year. While we are staking but could not spendable would you pay off all because you are changing blockchain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants