Bump ring size to 16 for v15 #8178
Conversation
|
I tried to review but bumped against a partial lack of knowledge: Is there an explanation somewhere about the meaning of unmixable and how a tx looks if I build it today that has such unmixable outputs? If not I would be glad for some short explanations; I tried to get the "mechanics" of unmixable / pre-RingCT / post-RingCT txs from the code alone but could not break through. My current understanding that has still some holes in it sees errors both in the "old" now-active code as well as in your new code, but that does not seem very probable ... |
Before RingCT, transparent amount outputs could only mix with other outputs of the same amount. Let's say back in the pre-RingCT days, I sent you an output worth 0.0004 Monero, and no other 0.0004 amount outputs ever came into existence in the chain (hypothetical). This 0.0004 Monero output would thus be "unmixable", since there wouldn't be 10+ other decoy outputs of the same amount with which you can mix this output. In this case, consensus allows you to construct a pre-RingCT transaction with a ring size of 1, where the 0.0004 amount output is used as an input, and pre-RingCT outputs come out on the other end. To simulate this, you can set up a test network where you mine a bunch of pre-RingCT outputs, then advance the hard fork to current, then call |
src/cryptonote_core/blockchain.cpp
Outdated
| // The only circumstance where ring sizes less than expected are | ||
| // allowed is when spending unmixable non-RCT outputs in the chain. | ||
| // Caveat: at HF_VERSION_MIN_MIXIN_15, temporarly allow ring sizes | ||
| // of 10 are to allow a grace period in the transition to larger |
|
@moneromooo-monero depending on the value of Also, instead of getting rid of the command to set the default ring size in this PR, what if I just change the logic of AFAICT, this ring size bump is the first one happening after the fixed ring size is already required by consensus, which is why I can't update the constants to the higher ring size in simplewallet (or leave them as is) otherwise people who start using the updated wallet before the fork height won't be able to construct tx's. That's why usage of the constants in simplewallet seemed worth updating in this PR (and I figured might as well just remove the command to set the default ring size instead of messing with that logic). EDIT: it seems like it would eventually make sense to move away from min/max ring sizes/custom ring size setting in the wallet altogether (and only use a constant default), but curious on your thoughts for this PR |
|
I meant that two very different things should be in different patches. I'm fine with the change itself. But I see your point now, I guess it's ok since there is a good reason, which I did not realize before. |
j-berman
force-pushed
the
bump-ring-size-16
branch
from
3a43d12
to
5188eae
Compare
j-berman
force-pushed
the
bump-ring-size-16
branch
from
5188eae
to
5d388eb
Compare
As per:
This PR follows the "double fork" pattern to allow ring sizes of either 11 or 16 during a grace period, and then requires 16. This way users of updated wallets who submit tx's right before the fork height that make it into the pool will still have their tx's processed.
Also removed the obsolete command to allow users to set a default ring size in the client. Updated wallets will start to use ring size 16 at the fork height.
I'm currently syncing the chain from scratch with this code as a sanity check. Currently on block ~2.3mn, which is after the latest fork. Will update here when finished. Update: synced from scratch successfully.