-
Notifications
You must be signed in to change notification settings - Fork 90
[WIP] RingCT Staking #678
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
[WIP] RingCT Staking #678
Conversation
|
|
||
| BOOST_AUTO_TEST_CASE(ringctstake) | ||
| { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Add test
|
|
||
| BOOST_AUTO_TEST_CASE(merkle_block_2) | ||
| { | ||
| // Random real block (000000005a4ded781e667e06ceefafb71410b511fe0d5adc3e5a27ecbec34ae6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random real block - comment is incorrect.
|
|
||
| BOOST_AUTO_TEST_CASE(merkle_block_2_with_update_none) | ||
| { | ||
| // Random real block (000000005a4ded781e667e06ceefafb71410b511fe0d5adc3e5a27ecbec34ae6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random real block - comment is incorrect.
|
|
||
| bool COutputRecord::GetKeyImage(CCmpPubKey& keyImage) const | ||
| { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be easy enough, the code already exists elsewhere in the code. Presstab was just making it more object orientated.
mimirmim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, RingCT is cleaned up to be more object-orientated and sane. Kernel is intact and fine. The block creation process needs to be done. Log prints of Zerocoin need to be replaced.
| size_t ofs = 0, nB = 0; | ||
| for (size_t k = 0; k < nInputs; ++k) { | ||
| const CCmpPubKey &ki = *((CCmpPubKey*)&vKeyImages[k*33]); | ||
| LogPrintf("Key image %d %s\n", k, ki.GetID().GetHex()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a temporary log print I believe, should be removed in final.
| //If this is a proof-of-stake transaction, then the minimum value must be set above 0 | ||
| uint64_t min_value = 0; | ||
| if (fProofOfStake) { | ||
| min_value = GetStakeWeightBracket(nValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's right isn't it. it's going to be the weight of the min of the bracket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to use something that the public will be able to validate. As they won't know the actual value, they will only have the min and max values that are in the rangeproof. For obvious reasons you would want to round down to the min value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got a solution for this, in fact, we are limited by how Rangeproofs are created. It's impossible to set a maximum value to be anything, it must be an even power of 2 (2^2, 2^4, 2^6 etc..). So, you can commit to a bottom of any value + 1 satoshi (so people can not stake in multiple denom brackets). The only real big change that kind of threw us for a loop was when we found out that someone can stake every bracket denomination. So, the only may to set a max is to actually limit the size of the rangeproof itself.
Through discussions with people that are more familiar with the libsecp256k1-zkp library, that's when we realised that all of the denomination and weights need to change entirely. We also realised that through some simple math we are able to quickly calculate what is the min max denom bracket values without making them static and having it apply to across the whole range of coins.
I wrote up some simulation code that is able to take in all the parameters that we are using and able to find which brackets are more unfair to others as the goal is to make them as equal as possible. You can see and play around with it on my github if you are curious. veil_pos_simulation. I've been working on adapting it with the new changes, then some of that code will make it into the Core library on how I did some of the calculations like the weights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@presstab thanks for jumping in. as @mimirmim touched on; the min/max ranges need to be defined by the number of bits that will define the private portion of a TXO value. The number of privacy bits will increase as the TXO value increases (by a logarithmic function). With that said, as we get into smaller denoms, the privacy bits will approach 1. This actually allows us to stake TXO dust. So there really isn't any need for a true minimum defined in the code. The algorithm defines the min and max that a TXO will fall into, and consensus can confirm that the staking transaction confirms to a min and max [ (min-1sat)*4 == max ]. The TXO value is within that range and the staking weight is the min.
I'm thinking of having a user configurable min_value. Let them define (or override) the default min_value to whatever dust size they don't want to have considered for staking; that way they know what TXOs will be considered, and won't be considered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any reason that a max value has any importance when it comes to bracketing/stake weights. The weight does not change the proof hash, so focusing on max value would not be a measure to prevent grinding.
Being able to "stake every bracket denomination" also doesn't increase the chance of finding a successful stake.
The formula for a winning hash is roughly:
success = hash(timestamp + Immutable UTXO Data) < (difficulty * weight)
If a stake is valid under one bracket, mathematically it is also valid under all bigger bracket levels as well. However using a bigger bracket would provide less reward, so would be against the interest of the staker and no real reason to do (unless looking for some additional obfuscation).
@CaveSpectre11 you don't want to be able to stake dust. At that point you would call it proof of work, not proof of stake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A big advantage to limiting the rangeproof size is its "cheaper" to produce smaller denom hashes since the size is reduced. Also, a bit less bloat on the blockchain which I consider quite a large win seeing as our blockchain size is exploding right now. It is incredibly wasteful to have these large rangeproofs when they are not exactly needed. It also shouldn't sacrifice privacy.
The last issue is that say someone does reduce the proof size. There is nothing that will ensure the proof size is of a certain length. So, I can have an advantage, more so than Zerocoin, of staking only 10 denominations (2^30) and having a minuscule proof size compared to 10,000 (2^38, if not changed from it's current 2^64), lowering my overhead computational cost and allowing me to have more capacity at staking 10 denominations than someone at the default 2^64 (which is beyond our max coin, so its wasting bytes on the blockchain per RingCT tx. Default should be 2^56 as bits must be even, else it would be 2^55).
Note: I haven't measured how much of a difference 2^30 v. 2^64 is yet, but from what I understand that the computational cost is substantially different after talking to some people familiar with rangeproofs on Elements with libsecp256k1-zkp.
@CaveSpectre11 you don't want to be able to stake dust. At that point you would call it proof of work, not proof of stake.
Nobody in their right mind right now would stake dust denominations right now, but in 10 - 20 years it could make sense to be profitable staking with only a few coins provided some issues happen like, someone potentially aquiring 60% of the coins and then forgetting the keys. You just don't know. Providing options that are future proofed to handle all scenarios is always a wise decision. We have discussed and will implement a minimum default value to ensure that nobody will be accidentally wasting their time staking denominations that won't hit a reward in years. Though if someone wants to stake dust, all the power to them. I wouldn't recommend mining Bitcoin with a RPI either but people try, and are not limited not to.
See:
I'm thinking of having a user configurable min_value. Let them define (or override) the default min_value to whatever dust size they don't want to have considered for staking; that way they know what TXOs will be considered, and won't be considered.
A huge issue with the current 10% penalties per denomination is that they are easily mitigated as an average computer can stake about 5,000 denoms easily without too much issue. This places most balances in the 10 or 100 denom values. We couldn't justify why they are as punishing as they are.
i.e, an average computer should be able to stake 5,000 100 denominations, or a value of 500,000 coins which is well well above which most people would have. There is almost no reason why you would stake 1,000s or 10,000s, contrary to popular opinion. (i.e my average 4 year old server only takes 3 - 5 seconds to stake that many denoms.)
Lets say that I had 50,000 coins. If I stake at 10,000s, that is 16+ hours (1000 blocks) that I'll be down 20% of my staking potential. 2% at 1,000. Ideally, you want to mitigate that to at least 100 denoms or even 10 denoms if you have an average computer that you are going to have staking running.
On edge cases, you should stake 1,000s. So, why have all these 10% drops per bracket if its easily mitigated if you are staking with more denoms? This is largely a fundamental change.
EDIT: Deleted tired brain response in the beginning. Shouldn't respond to posts at 4am >.<.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having smaller proofs that are less computational is a great thing. Still does not change the mathematics that a valid stake hash of a small amount of coins will still be just as valid with a larger max value bound. There isn't any real problem with that either, just saying that the important bound is the min not the max.
We have discussed and will implement a minimum default value to ensure that nobody will be accidentally wasting their time staking denominations that won't hit a reward in years.
I think you are missing the point. I could split a few thousand Veil into hundred of millions of dust UTXOs and then use a GPU to grind through all of those outputs and find a valid stake quicker than anyone else would be able to. If a system is designed to allow that, then that system is called proof of work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do get that and that is one of the biggest issues with PoS that you can split up your balances into many smaller UTXOs to have more permutations which would turn it into a PoW-like system (and something that I talk about a lot). This issue even exists no matter where the floor is provided with a large enough balance. A minimum value that you must have certainly can discourage this. However, that does bring in now a cost to actually create those coins.
i.e what's better? Rolling a many-sided die and having your result modified to provide you with a win if you roll a high enough number, or rolling that die many times as fast as you can until you get the high enough number. I've seen lots of people say the latter than the former and I do agree but, I need to see those results to be able to actually conclusively agree with that.
Once you throw in "But! you get a lower computational cost with smaller numbers!" then that kind of throws it into a loop where potentially an issue arises. However, there are other things like for instance, propagation times, who will build a block on your block before the other? and many other time-related issues. So, time aside, the probability should be about the same.
|
|
||
| bool COutputRecord::GetKeyImage(CCmpPubKey& keyImage) const | ||
| { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be easy enough, the code already exists elsewhere in the code. Presstab was just making it more object orientated.
src/wallet/wallet.cpp
Outdated
| listInputs.emplace_back(std::move(input)); | ||
| } | ||
|
|
||
| LogPrint(BCLog::BLOCKCREATION, "%s: FOUND %d STAKABLE ZEROCOINS\n", __func__, listInputs.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change ZEROCOINS to COINS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change
ZEROCOINStoCOINS
should be Change ZEROCOINS to TXOs, or something non denominational since it's not the total value, but rather total number of inputs
92f0523 to
7c17bcc
Compare
|
Changed from v2.0 milestone to v1.1. No RPC breaking changes. |
CaveSpectre11
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial thoughts
| auto nValueOut = txrecord->GetValueSent(/*fExternalOnly*/false); | ||
| auto nFee = txrecord->nFee; | ||
| if (nValueIn - nFee - nValueOut > 0) | ||
| LogPrintf("%s: Failed to get blinds for output %s %s %s valuein:%s valueout:%s fee=%s\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to change "blinds" to something resembling "stealth" since we changed from blind transactions to stealth transactions
| //If this is a proof-of-stake transaction, then the minimum value must be set above 0 | ||
| uint64_t min_value = 0; | ||
| if (fProofOfStake) { | ||
| min_value = GetStakeWeightBracket(nValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's right isn't it. it's going to be the weight of the min of the bracket.
| //Sets nValueIn with the weighted amount given a certain zerocoin denomination | ||
| void WeightStake(const CAmount& nValueIn, CAmount& nWeight) | ||
| { | ||
| nWeight = 0; | ||
| if (nValueIn == 10*COIN) { | ||
| //No reduction | ||
| nWeight = nValueIn; | ||
| } else if (nValueIn == 100*COIN) { | ||
| //10% reduction | ||
| nWeight = (nValueIn * 90) / 100; | ||
| } else if (nValueIn == 1000*COIN) { | ||
| //20% reduction | ||
| nWeight = (nValueIn * 80) / 100; | ||
| } else if (nValueIn == 10000*COIN) { | ||
| //30% reduction | ||
| nWeight = (nValueIn * 70) / 100; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this probably needs another calculator based on a gain rather than a reduction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, as we found out with the sims. Will be very different though with RingCT PoS since it can only get easier to produce a stake as the Rangeproof size is reduced.
|
OBE - See 1.2.0-Dev branch |
closes #616