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

Consensus: DigiShield (Initial work submission) #608

Closed
wants to merge 161 commits into from

Conversation

ChillingSilence
Copy link

Preface:
While I'm well aware this is not a complete / finished implementation and will probably fail to build happily, unfortunately I'm not a developer (As any experienced coder will no doubt be able to see from the commit), however this is intended to start the discussion around DigiShield being implemented for Litecoin.
I understand that thrasher- has begun work on 0.18 however this seemed the more appropriate place to begin that discussion, rather than on his own two 0.18 branches. This can be pulled into one of those branches once merged of course, rather than Master.

Motivation:
At the block reward halving at the start of August 2019, Charlie Lee rightly indicated that 12 blocks were found in 17 minutes:
https://twitter.com/SatoshiLite/status/1158326430574399489
By comparison, DigiByte at the same time had found 200 blocks in 49.5 minutes (Expected was 50 minutes, a 1% deviation), being far more steady / stable, closer to the expected timeframe.

For Litecoin, this is quite a variance over the expected 30 minutes for 12 blocks, with blocks being found almost twice as quickly as they should have been. A healthier network should have more stable block production.

Charlie Lee has also recently discussed Litecoins block timings in late July, accurately recalling how the initial 10,000 blocks for Litecoin were mined in the first 24 hours of Litecoins existence. Although DigiShield was not around for Litecoins creation in 2011, because a lot of new blockchain projects are still to this day basing their blockchains off the Litecoin code-base, we can also "save them from themselves" by having DigiShield implemented in what becomes their "upstream". A win / win situation for all.

DigiByte has been utilizing DigiShield since Feb 2014, with Dogecoin also implementing DigiShield (Readers of this are no doubt aware that DOGE is merge-mined with LTC) following consultation with (and assistance from) the DigiByte developers at the time. DigiShield is presently used in a couple of dozen other major blockchains at this point in time in 2019.

There is no down-side to a more stable and steady block production schedule, and this only impacts block emission timings, and not the block-rewards in any way. The block timing will still remain 150 seconds (2.5 minutes), unchanged. This will simply be staying closer to it.

Proposed using version bit 2, nStartTime = 1569888000 being Oct 1st 2019 is approx 3 months away with a 12-month timeout to achieve consensus on the upgrade.
Chose Block 1,770,000 for fNewDifficultyProtocol as this is 6 months in the future. Theory being it should provide sufficient time for miners to upgrade even if the consensus is attained earlier, while also not being too far in to the future. Because there is no down-side to this DigiShield upgrade, and it also helps miners with a more reliable block timing, I don't see why this would be a controversial upgrade.

Additional work is almost guaranteed to be needed to finalize the implementation, add additional tests for testnet upgrade etc but as mentioned this is a starting point that I hope is received in good faith with #LoveFromDigiByte.

laanwj and others added 30 commits August 13, 2018 15:34
Update version after branching off.

Tree-SHA512: a2e2f82e7b50c0027edc8f382ad29429327edb81ca058abe3c35e049afcd5b7bcedae4545bfb59e8f7a1c8b9c97cfe550d8ae9275bd9e90cfadea22f7b835a2b
f0f745d doc: regenerate manpages (fanquake)

Pull request description:

  Done using: `contrib/devtools/gen-manpages.sh`

Tree-SHA512: 523b333768aa5ff289ceee3dbe627698f60c7b4624a1fe2812a40d99a5184aa2f6abd20fa467487918bbcbe1d88693c589456e75a5e747281333f75ab1f5f8b9
0-input transactions can be ambiguously deserialized as being witness
transactions. Since the unsigned transaction is never serialized as
a witness transaction as it has no witnesses, we should always
deserialize it as a non-witness transaction and set the serialization
flags as such.

Also always serialize the unsigned transaction as a non-witness transaction.

GitHub-Pull: bitcoin#13960
Rebased-From: 43811e6
… as witness

Strip out the witnesses when serializing the non-witness utxo. However
witness serializations are allowed, so make sure we always deserialize
as witness.

GitHub-Pull: bitcoin#13960
Rebased-From: bd19cc7
0333914 More tests of signer checks (Andrew Chow)
8935869 Test that a non-witness script as witness utxo is not signed (Andrew Chow)
dbaadc9 Only wipe wrong UTXO type data if overwritten by wallet (Pieter Wuille)
ad6d845 Additional sanity checks in SignPSBTInput (Pieter Wuille)
517010e Serialize non-witness utxo as a non-witness tx but always deserialize as witness (Andrew Chow)
8c4cd2b Fix PSBT deserialization of 0-input transactions (Andrew Chow)

Pull request description:

  Backports bitcoin#13917 and bitcoin#13960 to the 0.17 branch.

Tree-SHA512: b3853aff2a13a53aa0a390b6b4b0c539f0ef0d42f2c517e956efd0b135c74c4ddce6a1d00700849a58c696824fa95951d8cac6ca58b426e8dfcb8bb62f680b7c
Tree-SHA512: 11d0d6a23f47e428661b33fa175aa97cc6841452c0c55845fdb0a903a0b147cd6df65e8fdab8b98823bf411018d5d85006af8c2cf14597286e9e284764d15041
Qt's configure grabs the path to xkb's data root during configure, but the
build changes in 5.8 apparently broke the handling for cross builds. As a
result, the string embedded in the binary depends on whether or not some files
are present in the builder's filesystem.

The "-xkb-config-root" configure setting is intended to allow manual overriding
but it is also broken. See: https://bugreports.qt.io/browse/QTBUG-60005

This has since been fixed upstream, so just hard-code the path for now. We can
drop this patch when we bump to a fixed Qt.

Also, fix the "-qt-xkbcommon-x11" config param which was renamed. This does not
appear to affect build results, presumably because auto-detection is working,
but it does not hurt to be explicit.

Github-Pull: bitcoin#14000
Rebased-From: de0b4fb
48c8459 depends: fix qt determinism (Cory Fields)

Pull request description:

  Backport for rc2

Tree-SHA512: 990a1b32ca8c80b22595c4b19e801d5033e42b7e86da0f80028e708131a136f6383d7510dab8bd7cd12fe8533f73246fe78c72a8a22a54eb136b2bfada1e67ea
Github-Pull: bitcoin#13968
Rebased-From: 2252ec5
Tree-SHA512: 1f9978ee25fbe9bb338af19d81b401fa531d314ac8288cdb21c1bf10459edea50b43e2d5e97c9bb5fe24c8db89363f8233c0a3d96066ed85f7bd6d2eb234aac0
Github-Pull: bitcoin#13968
Rebased-From: 1f18d7b
Tree-SHA512: 90391703181db6880a135c60aca792a9e92c4abcad26907cd6cb0a0378593fe45cf995a22ae142ea7de2767c72a9df444e918ff15e460ce19c0435163917d812
Github-Pull: bitcoin#13968
Rebased-From: 1f0c428
Tree-SHA512: 1f8b10629a314f623d589801ef2e62724de2cd82a0e523e0ab25a285f92b76a3b31304c1c0418b1b611ec3ca0016016d1e6af410ac81a78449b875c4f89a46d7
Github-Pull: bitcoin#13968
Rebased-From: faaac5c
Tree-SHA512: 758c0c3e4435897d1a9b03ea93f1b2a1a1b64071eda9450f968acf537c172ee61acf9d962bc22ddb6de26e0ad39d9165cdee6f260bb5a95bf97b4003853f0874
Otherwise, the generated Makefile is included in the NSIS-installed documentation, which can lead to non-determinism (eg, if gawk is installed on some build VMs, but others only have mawk)

Github-Pull: bitcoin#14018
Rebased-From: 8563341
Tree-SHA512: 2d219a4a2027bcd7359b7320bafc6b7cd3bde3dcf9309ddd6198ff67407470025baa71e6d0ed3d6cec081834ddc9a0247043865eb26737e6fd0d2f09574f5932
cf3d7f9 Use assert when running from multithreaded code as BOOST_CHECK_* are not thread safe (Jesse Cohen)
fab0fbe qa: Stop txindex thread before calling destructor (MarcoFalke)
b5ec6d4 Docs: Fix help message typo optiona -> optional (Ben Woosley)

Pull request description:

  Fixes to make the unit tests and bench pass with the thread sanitizer (beside the issue with fChecked bitcoin#14058 (comment)).

  For testing: `./configure --with-sanitizers=undefined,thread && make -j 16 && ./src/test/test_bitcoin`

Tree-SHA512: 5cb85ecc278b719dba03240265e93424ed1a28671834da7590adab88c2d43c6e6cbf3269bbe2fd79e5ed3a85ec77a268e05301e7a7421cf6a97d413dddac6327
Github-Pull: bitcoin#14055
Rebased-From: 61fe653
Tree-SHA512: 2f3edf62318fab4b405b47788096005f59cbe6ba4723fe51ce3b386539a58b7ea7369c31c3840c6baa76cdf6ba8f8440f977c36e2ee2916e711d7872bd1eadad
PR bitcoin#12713 changed the interpretation for negation of non-boolean options
(e.g. -noconnect) to no longer set the option to 0, but to remove it
from the options.

I think this is better because it gets rid of the special meaning of
'0'.

However it needs to be documented. I attempt to do so in this PR.
Addreses bitcoin#14064.

Github-Pull: bitcoin#14100
Rebased-From: e9a78e9
…boolean options

6bfee8a doc: Update v0.17.0.0 manpages (MarcoFalke)
2936dbc doc: Change documentation for =0 for non-boolean options (Wladimir J. van der Laan)

Pull request description:

  Github-Pull: bitcoin#14100
  Rebased-From: e9a78e9

  Includes the bumped manpages.

Tree-SHA512: 73d2dadb45418882122313975c0ab0e9f58310697d996dd2edeb400ebe73b3a45f1157c8a7fe65ae1f53d9ce68a88aae7c701f3e82e0b4db4c9417b36ddfecc0
Report errors while parsing the configuration file, instead of silently
ignoring them.

    $ src/bitcoind -regtest
    Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
    $ src/bitcoind -regtest
    Error reading configuration file: parse error on line 22: sdafsdfafs
    $ src/bitcoind -regtest
    Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -

Github-Pull: bitcoin#14105
Rebased-From: a66c0f7
Tree-SHA512: 2b6be1ab643623e6ef9b53354820147a6c5d2baae3795ffe428fc60d8563ec00a68a379aee4029380f80f892abe23763afb1c75c32b60a13bffe7b82496bf2bb
Github-Pull: bitcoin#14105
Rebased-From: ed2332a
Tree-SHA512: 17fa88a2848f1c9c9c8a127b5ea4c45761ce8e06a609dd40f8e90bb9117d88c9d2c81e752c9c0f1a44ecadbb5bedd2973bc4548da2a6d463c789797191e85ab1
Github-Pull: bitcoin#14096
Rebased-From: 9254ffc
Tree-SHA512: 1fc466177dbe3b57b0025c102b1e75e4b05544440819885da7a85b70d20329fc3f6f4cbf89d7d6a48881e6ad176d81f7396f02925586140e19468a2f28f3414e
Github-Pull: bitcoin#14096
Rebased-From: 89709db
Tree-SHA512: 0bf74e1179bee6e616d8fd2c125411ef30611d9aa3b32412025615a793abbc629a7b6d89a89ef05b11ac3541cc869c0caebba2d00942fdd6ab8d2e98d9b9caca
@ChillingSilence
Copy link
Author

It's been brought to my attention that I should probably explain better the purpose of DigiShield.

DigiShield is a dynamic real-time difficulty adjustment method.

Every block, it readjusts the mining difficulty, keeping the timing as close to the intended emission (mining) schedule as possible, compared to the traditional 3.5 day adjustment period. This is especially important around times of block reward halvings due to the fluctuation of hashrate in and out. However, it has been broadly proven to offer a more stable block timing, especially for a blockchain that's the dominant hashrate for it's algorithm such as Litecoin.

This is beneficial for miners too, as they're less likely to spend longer mining a block for the same reward, if the emission time is more steady.

DigiShield does not affect the block size, change the block timing, throughput, speed, or anything else. It simply makes the block timing more reliable.

Thank you to Rob D (@BoilingPointLTC) for pointing out the needed clarification, really appreciate the feedback, and the support.

@FaysalM
Copy link

FaysalM commented Oct 22, 2019

@ChillingSilence - i cant speak for how @coblee or @thrasher- would view this. But just from a classic view on how blocks are produced, the fact that we had 12 blocks in 17 minutes is a feature, not a bug. It tells us, the difficulty adjustment is working as expected. Yes this causes some headaches during very high periods of difficulty adjustment, but thats not quite a reason to jump ship.

One of the reasons to keep the block adjustment as-is would also be the fact that a lot of multi-mine pools switch through a lot of scrypt coins which are all essentially litecoin clones. This was a bigger problem in the past, I am not sure how much it is prevalent today. We want to encourage longer term miners more than opportunistic mining. This adjustment would make much more sense for the coins with lower hashrates, that try and compete with the litecoin hashing. imo doesnt make a very good case to go into ltc.

@ChillingSilence
Copy link
Author

The 12 blocks in 17 minutes isn't difficulty adjusting anything though, the difficulty is still the same between all those blocks. The unexpected faster block timing only gets taken in to account at the time of the next difficulty adjustment, and it will slow things down by increasing the difficulty, which had not occurred at that stage.

If there is more hashrate coming in and going out from merge-mining, that's again a perfect opportunity for it to smooth the block timings, as DigiShield was originally created back in 2014 to prevent multi-mining pools from stalling the chain (This is what occurred with DOGE, which is why they implemented DigiShield).

@coblee
Copy link
Member

coblee commented Oct 22, 2019

NACK

This requires a hardfork, so it's unlikely to get adopted by Litecoin. Litecoin, like Bitcoin, has dominant Scrypt hashrate, so it doesn't need per-block diff adjustment like other coins do. I disagree that there aren't any downsides to Digishield. For one, it requires a hardfork, which is a huge downside. Second, it makes mining earnings less predictable as @FaysalM hinted at. It makes miners have to do work calculating profitable at every block instead of ever 3.5 days. Lastly, this opens up potential new attack surface against mining. For example, miners can set their block time to try to mess with the difficulty. When everything is average out in 2016 blocks, it's much harder for a small subset of miners to cause problems.

@ChillingSilence
Copy link
Author

Thanks for the response.
I will agree the down-side is a hard fork, it's worth being mindful of for sure. If that is the show-stopper, perhaps this is worth keeping in the back pocket for such a time as another hard-fork occurs, if that is the last remaining concern? After all, Litecoin was able to gain consensus on SegWit, I'm sure it could put it to the miners to signal support (or not) for this too.

However, stating "We're the biggest so this won't apply to us" is a bit misplaced, especially given Ethereum is the largest in Ethash, yet they utilize a difficulty adjustment algorithm based off DigiShield and adjust every block.
Same goes for ZCash being the largest Equihash blockchain, they too use DigiShield.
DigiByte is also dominant in Skein, Qubit, was previously with Myr-Gr, and is now also with Odocrypt. I could go on. However, not being the "dominant" hashpower is not the sole (or even primary) reason to want it. In fact it was specifically not the reason it was suggested for Litecoin, but rather the unpredictable block timings would be made better.

That's the crux of the suggestion: Block timings on LTC are significantly less reliable currently than they would be with DigiShield.

If the miner reliability of earnings is truly your concern as you have stated, then if the blocks are more stable / regular (which means a steadier block reward for miners) then miners income is actually more predictable with DigiShield. Contrary to what has been stated about the earnings being more predictable with the current method, having 12 blocks in 17 minutes (instead of the intended 30mins) means regardless of if that was luck, random chance, or an influx of hash power (sustained or not) the next difficulty adjustment will have to compensate for that and further exaggerate the difficulty at next adjustment to throttle the block timings.

Unrelated to this pull request, but I have put together some graphs to visualize the difference between the current for BTC / LTC, and DigiShield. It's quite pronounced: https://medium.com/@josiah_digibyte/measuring-expected-block-timing-for-difficulty-adjustment-c7a5bbffa775
I'll include the graph here, you can view the link if you want more specifics
image

As for additional effort for miners adjusting difficulty every block, the computation there is negligible at best. We have the likes of Ethereum doing it successfully, yet having a far more distributed mining network and also significantly faster block timings. Same for DigiByte. I don't see why this would suddenly be an issue for Litecoin, with 2.5 min block timings? Can you shed some light on that for me please?

I also don't quite understand what you mean when you mention "miners setting their block time to mess with difficulty", can you clarify what you mean by that? The difficulty is not based on time or the unix epoch, so if you're talking about what has happened previously with the likes of Verge to mint blocks, that's not applicable to DigiShield. That said, if you are aware of attack vectors with DigiShield that DigiByte, Ethereum, ZCash etc are unaware of, I'm sure all projects would welcome your insight.

Thank you for your time and input :)

@CaptThomas
Copy link

"This won't affect us because we're the biggest"
Aged like fine wine

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.

None yet