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

[RFC] Stake Announces based on Locked Transactions discussion #176

Closed
bitkis opened this issue Dec 10, 2018 · 4 comments
Closed

[RFC] Stake Announces based on Locked Transactions discussion #176

bitkis opened this issue Dec 10, 2018 · 4 comments
Labels
RFC-draft Request For Comment -- draft

Comments

@bitkis
Copy link
Contributor

bitkis commented Dec 10, 2018

RFC 001 SALT -- please provide your feedback

@bitkis bitkis added the RFC-draft Request For Comment -- draft label Dec 10, 2018
@jagerman
Copy link
Contributor

I've got a lot of comments about this proposal, as you might expect from me. Overall, this is a step in the right direction for handling stakes, but it still leaves some major issues that must be addressed.

Replies to specific details

Stake Transaction (StakeTx) is a regular Graft transaction with the following modification:

  1. The receiver of the transaction is a sender.

This is impossible to verify by anyone except the sender. It also seems pointless: there seems no reason to not allow a staking transaction to also be a time-locked transfer from one wallet to another. What would be gained from such a restriction?

  1. Amount of transaction must correspond to a supernode tier.

There are many situations where this might not be the case: if a staking tx needs to be split (because it was sent from a wallet with many small outputs); to support shared stake supernodes; and to upgrade a node (e.g. from T1 to T2) with an additional "supplemental" stake.

  1. tx_extra must contain the temporary public identifier key.

I don't understand why it is temporary; more comments about this below. Moreover it seems like considerably more information than just the public key is needed: you need some data which contains the public key, the recipient wallet for RTA fees, potentially a block height, and then a signature generated by the private key signing the data (instead of point 4, below). For example, the data might contain:

{ "data": "{\"pubkey\":\"abc123...\",\"recipient\":\"G8MyWallet123...\",\"expires\":123456 }", signature: "...signature of data value by SN private key..." }

which can simply be included in the tx data. There is no reason at all to generate this data without a signature, stuff it into a transaction, then sign the entire transaction instead of simply signing just the SN data in the first place.

I think that the reason for the ordering here was an assumption that staking will be initiated from the supernode itself, but I will, further down, strenuously argue that this is neither necessary nor desirable.

  1. The signed transaction must sign with the temporary private identifier key (which corresponding to temporary public identifier key.)

See above: this is unnecessary step and adds a complication for no gain.

Two important factors are also missing here:

  • Why are you not mandating a time-lock requirement? This is not a small piece of the system, rather this is a major part of the incentive to run a reliable supernode: if the supernode goes down, your stake is still tied up for the duration of the stake period. That gives you an incentive to put extra effort into keeping it up.

  • The transaction private key must be included in the transaction extra data. It means StakeTxes leak a good deal of their privacy, but that's absolutely necessary: you need stake amounts to be publicly verifiable.

Lock Validation Further investigation on the locking transaction mechanism is needed, including studying other projects that have dealt with this issue. (adding expiration information inside the transaction might be needed)

What? Am I misreading this? What's wrong with the core transaction unlock time mechanism that has been in cryptonote since the very beginning?

Stake state of a new supernode, which wants to become a stake supernode, should be validated. To support such validation, the supernode needs to generate a self-addressed Stake Transaction with the amount that corresponds to a specific supernode tier.

This seems pointlessly complicated. Why not put all the information into the (single) stake transaction submitted by the wallet contributing the stake? There seems no reason for the SN itself to have to generate this transaction.

tx_extra field of the transaction must contain a temporary public identifier key, generated from private keys of the supernode's wallet. The supernode must sign the StakeTx twice, the first time using regular wallet keys, and the second time using temporary private identifier key.

This seems needlessly complicated, and requires the SN maintain more information than should be necessary. What is gained by this approach versus simply using a non-temporary (i.e. permanent) SN-specific key (not one associated with a wallet) + RTA payment recipient wallet and private view key (so that the SN can verify RTA tx amounts)?

After signing the StakeTx, the supernode sends it to the transaction pool, ...

Why is this happening on the supernode? It seems as though all of the necessary information (i.e. the SN public key and block height) can be pre-generated on the supernode and then simply included in the StakeTx generated and sent from a wallet to a node anywhere on the network. Requiring that it go back via the same supernode to be signed yet again seems like a complication that gains nothing at all in security, but makes registration more of a hassle.

... generates and broadcasts an announce, ...

Why is this announcement needed? It seems, given the details below on handling such an announcement, that it is virtually always going to be ignored since the StakeTx is only in the mempool and not on the chain. More on this below.

... which contains temporary public identifier key ...

Why is this public key temporary? What is its lifespan and why does it need to expire? Since the stake is tied to it, it seems as though it can never expire, so what is the point of it being temporary?

... wallet public address, wallet private view key and current block number.

Broadcasting the wallet private view key has to be dropped from the design: it is a massive privacy leak of the entire RTA system. If I log view keys included in announces on a network node I can use them to determine the transaction amount of every RTA transaction that goes through the network. This is trivially easy to do: just load up all the supernode view keys and watch for RTA transactions deposits into those wallets, then multiply the received amount by 1600 (or 800 on the current 4-sample testnet) and you have the transaction amount.

Moreover the private view key isn't even enough to verify that RTA transactions include the correct amounts (more in the general comments section below).

Constants: N = 20 - the number of blocks needed to approve StakeTx{info}

What is the point of this 20-block StakeTx confirmation? Spamming the network with registrations should be prevented by choosing a good minimum stake period. Adding a delay here seems to have no rationale.

An N-block delay also pointlessly penalizes stake renewal with a 20-block downtime: what is the purpose of penalizing someone who has been running a reputable supernode for 20 blocks when they want to continue operations by renewing their stake?

Each supernode A checks if StakeTx of supernode B is in the blockchain upon receiving an announce from B.

This emboldened part seems beyond strange. What is the point of using network announces for this rather than simply basing validity on the blockchain itself? The latter would be far more robust and eliminate a good chunk of unnecessary network traffic.

Stake Supernode Initialization

During initialization, ...

Why is this section of the RFC here at all? It seems to be identical to the Stake Supernode Periodic Announcement section that immediately follows it.

Stake Supernode Periodic Announcement

[Periodically] a supernode checks if its StakeTx is in the blockchain. If no StakeTx found:
2. ... supernode ... create[s] a new self-addressed transaction ... Sign generated StakeTx, add signature created using temporary private identifier key and send StakeTx.

Why is this part of the supernode server design at all? Stake handling should be done by a totally separate wallet process that can managed entirely independently of the supernode itself: it could be done on a cold wallet only brought online when a stake reaches expiry, or on a private network completely isolated from the supernode. Requiring that the supernode has the private spend key is a huge security risk that community members have resoundingly said that they do not want.

  1. If the supernode doesn't have enough balance, it cannot participate in RTA transaction validation but still can be a proxy supernode. For that, it needs to generate announce request with the following fields: wallet public address, blockchain height, and broadcast it to the network.

Is this optional? Is it required? Is a supernode with an active stake required to operate as a proxy SN? If so, when did this become part of the supernode requirements? If not, why is this part of the RTA announce design at all? "Your SN can (or most?) proxy requests" seems to have nothing to do with having a valid stake or being a valid RTA supernode, so the inclusion here is strange.

Announce Signature Each announce should be signed by stake private keys. Which allow validating announce for any supernode.

It makes no sense to sign these messages with the private key of the stake wallet. You have generated a separate private/public key for the supernode, why don't you use it?

Supernode Announce Handling

The design of this whole section is flawed from the beginning. Supernode validity needs to be tied to universal states the blockchain, not to announces. An announce here seems to serve no purpose except to say "go check the blockchain." Why can't a supernode simply check the blockchain when a new block is received? This seems like an unnecessary part of the design that serves no practical purpose.

  1. The supernode checks that the announce contains temporary public identifier key and private view key.

This is missing a huge, critical component: the supernode needs to check that the announce is also signed by the private key associated with the public key. Additionally, the private view key should not be here (see above discussion about this being a huge privacy leak, and the discussion below about why the view key is totally unnecessary).

  1. Supernode A checks how many blocks were generated after block which contains found StakeTx. If there are less than N blocks, A adds B's data (wallet public address, wallet private view key, temporary public identifier key and blockchain height) to Waiting Full Supernode List. Otherwise, it goes to the next substep.

As discussed in points above: the N-block delay, the "Waiting List", and the private view key should not be here. (It's also very odd to include implementation details — like using a waiting list — in a design document).

Full Supernode Lists Management

This is just describing how to manage a data structure to cache the information on the blockchain. It seems entirely pointless to include it in a document of this scope. Please delete it and stick to the point.

The one thing that is crucially important here—maintaining the list of supernodes in such a way that the list is always consistent across the network—is missing.

Stake Transaction Blockchain Validation

To validate StakeTx, graftnode must validate all key images,

This step is exactly nothing. Key image validation already had to happen for the tx to be admitted onto the blockchain. If it's on the blockchain the key images are already validated.

and check signature, added to StakeTx, has been generated by temporary private identifier key, corresponding to temporary public identifier key contained in tx_extra of StakeTx.

This is missing some important details:

  1. StakeTx SN data validity should be tied to the block height. I suggested, above, an expires field which is set to a block number: the StakeTx would only be valid if included in a block with H <= expires. Expiry of a week (H + 7*720) would be a reasonable default. For people who want to use some sort of auto-renewing stake it might make sense to allow them to specify a much longer value (e.g. 1 year).

  2. Supernodes (likely with the help of the node) need to be alt-chain aware. If a blockchain reorg moves a StakeTx back into the mempool then supernodes also need to recognize this invalidation while simultaneously recognizing any new StakeTxes along the new chain.

General comments

Atomic time

The time unit for a blockchain is a block. This may seem obvious, but many of the assumptions here—particularly the ones around managing the list of active supernodes—are assuming a different atomic time unit. When you need to synchronize information across time, that simply doesn't work.

No need for SN stake wallet keys

Everything described in this proposed approach can be done with a generated SN public/private key and the public recipient wallet that should receive the supernode's earning. Restaking, as mentioned above, should not be part of the supernode core, and so the stake wallet keys are not needed for that.

There are other drawbacks, as well, in assuming a 1-to-1 relationship between supernodes and tiers: someone should be able to stake more than one supernode from the same wallet; and shared stakes means a supernode "recipient" may be multiple addresses rather than just one.

Moreover having SN private view keys is not sufficient for RTA transactions: with the recipient view key there is no way to know whether the correct amount has been sent. The current supernode code just accepts any RTA transaction that paid it something. So the view key here doesn't even help. It turns out that you don't need the SN private view key at all to verify a payment: you just need the tx private key, the RTA recipient public address, and the SN public address. That's it. You specifically don't need even private view keys (much less private spend keys).

Of course you do have to keep these details confidential: the generated SN authorization request needs to encrypt both the recipient address and the tx private key using each SN's public keys and send these 8 requests out to each of them.

But note that this entire plan does not require any private keys (not even the private view key) of the wallet staking the supernode.

Now that you have (wisely) thrown out live balance verification for supernodes, it's time to also through out the extra junk that was only needed for live balance verification which will massively slim down the requirements for running a supernode.

Uptime announces

Announcements are still needed, but they are needed now only for the supernode to prove to the network that they are still alive. This proposal contains no details of how this will work. (In fact, it doesn't currently have any practical use for announcements at all, yet still sends them).

Node deregistration

In the proposal as written, the only way a node ever stops being registered is if it reaches the end of its stake period. This provides zero incentive for reliable operations: if my node goes down there is no penalty whatsoever.

Node deregistration needs to happen based on when a node has communicated its uptime; if it hasn't communicated one for a certain amount of time (for example, an hour) there needs to be a mechanism that removes this node from active supernode lists.

It cannot, however, be done at the individual node level: the information must be synchronized across nodes through some SN consensus mechanism either by recording the deregistration on the blockchain, or by some other SN synchronization mechanism. The reason is as follows:

Supernode list consistency

In the RTA network design amendments above, there is still not a consistent list of supernodes across the network. Supernodes only reconsider whether to add a supernode when receiving an announce, and only consider when to remove it on some pre-defined interval. Since these aren't perfectly synchronized across nodes, that means nodes are regularly going to disagree about the composition of the set of active supernodes on the network, and thus potentially disagree about the auth sample.

Disagreement about the auth sample is a huge problem that must never happen. The fundamental problem is that if it can happen then you can't really trust the auth sample: I could set up my own proxy SN that always uses my own 8 supernodes for an auth sample. With a totally consistent auth sample list you can stop the network from accepting such transactions because they used the wrong auth sample; with a consistent auth sample you basically have to allow anything.

Temporal consistency

The design is missing another, related fundamental piece: it needs to be able to ask "what was the valid auth sample at height H?" This is necessary because RTA transactions include an auth height, H, at the time the transaction begins which is used to generate the auth sample. But if you want to be able to validate that auth sample you need to be able to know 100% consistently what the auth sample was for that height -- even if the network has gone ahead by 5 blocks and new SNs have become active (or existing ones inactive).

There is no temporal component to the SN lists in the current design: they are simply either "waiting" or "active" (and waiting serves no point at all -- it is simply an information caching implementation detail that doesn't belong in this document). This means it is impossible for someone to ask "what was the auth sample at height H?" because all they can really ask is "what would the auth sample had been at height H if the currently active supernodes had been the currently active supernodes at height H?"

This needs to be fixed so that the network can legitimately disallow RTA transactions that are not signed by the correct supernodes from entering the blockchain.

@softarch24
Copy link

@jagerman thank you for detailed feedback! Working on answers and updates

@bitkis
Copy link
Contributor Author

bitkis commented Dec 27, 2018

@jagerman,

We've made a lot of changes and reviewed not only Stake Announcement Proposal but complete supernode management and RTA flow. Your comments and suggestions helped us a whole lot, we adopted most of them in the updated design. We're sincerely grateful for your attention to the details and your criticism.

Stake Transaction Design

We decided to use transaction private key for stake transaction validation and allow to send stake transaction from other wallets to the supernode wallet address. However, we still require amount to correspond to a supernode tier. Also, this RFC did not contain additional details about locking requirements, so we add more details now. Atomic time of stake locking is block, and we'll add the number of blocks for transaction locking inside the transaction. Particular values for stake locking period will be finalized later and possibly become part of the whitepaper since it is a very important part of the system.

These changes allow completely remove the full wallet and the private view key requirements, so stake transaction submitting and renewal will be done from CLI or GUI clients.

For more details see the new proposal drafts [RFC-001-GSD]-General-Supernode-Design and [RFC-002-SLS]-Supernode-List-Selection. Please keep in mind we're still working on those.

Stake Validation Downtime and Stake Renewal

20 block mentioned in this RFC was the fictitious number, while particular value of this strongly depends on network stability. However, we still think it is required to prevent side effects in the case of blockchain reorganization or 51% attack. We also think that downtime during stake renewal is unwanted, so we added trusted re-staking period to prevent supernode downtime. Please see the new proposal for more details.

Temporal vs Permanent Supernode Identification Keypair

You asked why we did not declare permanent supernode identification keypair. The main reason was that we didn't see any reason to make it permanent. The temporal keypair is enough for our goals and regeneration of this key won't create large overwork during stake renewal. And yes, the lifespan of this key pair will be equal to the stake period and during stake renewal supernode owner also need to update it. If someone wants to build a tracking system, they can do it anyway.

Announcement Mechanism

At the first place, the announcement mechanism is used to build communication channels between supernodes, as well as a heartbeat mechanism for supernodes. For more details see the new proposal.

Supernode list consistency

Changes regarded to the stake validation also allow us to change supernode selection algorithm and make it more consistent. Although, we still don't think we can be based solely on blockchain history -- stake transaction cannot indicate if its supernode is online or not, and this knowledge is crucial in case of RTA. Also, although the auth sample history tracking is important, but it does not cover all our requirements. So we thought of auth sample based on RTA payment identifier, which allows the increase chance for supernode to participate in the auth sample and also increase randomization of auth sample selection in time. For more details, please see the new proposal.

@bitkis
Copy link
Contributor Author

bitkis commented Dec 27, 2018

If you provide your feedback to the new RFCs, feel free to open new issues, and I'll mark this one closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC-draft Request For Comment -- draft
Projects
None yet
Development

No branches or pull requests

3 participants