Discussion: Should a node be able to enroll multiple times with the same address ?

[Geod24]

Currently, our pre-image scheme relies only on the private key, a nonce, and a fixed string.
This leads to a situation where a user would get the same pre-image if they enrolled with two different frozen txs controlled by the same private key. This might not be wanted, or this could be a feature. It's not an issue with the protocol, but rather our implementation. Thoughts ?

CC @bpfkorea/core

[linked0]

How about using the HD Key generation concept?

In the above picture, we can use the child keys of the first child as the seeds of random seeds. One child has about 2^31 number(2,147,483,648) of child keys so each height can have the seed or private key for initial pre-image which is described as the random seed in the white paper.

It also makes us not need to look through all the blocks in order to restore the cycle index for the next random seed.

[linked0]

The idea about how to apply HDKD(Hierarchical Deterministic Key Derivation) to AGORA

• We could use the public keys of children of the first child of master keys as the key to an enrollment.
• We could use the private keys of children of the first child of master keys as a component for making a seed of pre-images. Currently, we use the private key of nodes for it.

Docs about HD wallet

• What the HD wallet is (part of the book, "Mastering Blockchain")
• Why the hardened derivation is needed

Common industry standards

• Mnemonic code words, based on BIP-39
• HD wallets, based on BIP-32
• Multipurpose HD wallet structure, based on BIP-43
• Multicurrency and multiaccount wallets, based on BIP-44

Best Practice
As a best practice, the level-1 children of the master keys are always derived through the hardened derivation, to prevent compromise of the master keys.

[linked0]

The libsodium supports key derivation from master keys and it's similar to BIP-32 but not the same as the standard. But we can start with the functions of the libsodium.

Key derivation with libsodium >= 1.0.12

Part of Bitcoin's implementation is as follows.
CPubKey::Derive

[linked0]

We can use a child public key as an enrollment key and a child private key as a seed for making pre-images as follows.

[linked0]

In order to implement deterministic wallets completely, we should implement many standards as follows.

1. HD wallets, based on BIP-32
2. Multipurpose HD wallet structure, based on BIP-43
3. Multicurrency and multi-account wallets, based on BIP-44
4. Mnemonic code words, based on BIP-39

As the first phase, we start implementing HD wallets and we can refer to the bitcoin codebase. We can see the related codes of the Bitcoin as follows.
Key Derivation
Generating the private key and chain code

As the second phase, we should implement BIP-43 and BIP-44 which are related to making a multi-account wallet. The standards are about managing and discovering the multi-accounts.
Account discovery in BIP-44
Related implementation in Bitcoin

It would be needed to register AGORA coin types for BIP-44(See registered coin types)

As the last phase, We could implement the Mnemonic code words base on BIP-39, but we should discuss later how much we need the feature.

As part of this issue, I start implementing HD wallets. If you have any other opinions about this, please let me know. And other items to be implemented will be spit into other issues.

[linked0]

Does the requirement contain the feature that a validator can enroll with another UTXO at any height or is it just to make the validator be able to re-enroll another UTXO?
If the letter is the requirement of this issue, we could make use of the cycle_index by increasing the value for another cycle whatever UTXOs a node uses for enrollment. If the first is our requirement, we should consider many things like managing multiple PreImageCycles.

@Geod24 Please let me know your opinion regarding this.

[Geod24]

The former. But I don't see why we would manage multiple PreImageCycle. The idea is that it can happen in two instance of the agora program which don't know anything about one another.

[linked0]

The reason why we should manage multiplePreImageCycle is that if we use a UTXO hash as a component for making the cycle seeds, we have multiple cycle seeds for different UTXOs.

[Geod24]

But a single Agora process can only handle one enrollment (== one UTXO) at a time.

[linked0]

Ah, I see. Thanks.

[linked0]

But a single Agora process can only handle one enrollment (== one UTXO) at a time.

We need to consider that there's no way to determine which UTXO is for the single process in the catch-up process. But we could make the user select one. In the implementation of this issue, I can not help selecting the last enrollment of the enrollments found in the latest 1008 blocks, I guess.

[Geod24]

@linked0 and I had a few talks about this this sprint. His original approach was interesting, and had other benefits, but much more advanced than what the scope of this issue requires. There was supposed to be a PR with a simple fix however I do not see it in the PR list.

[linked0]

@linked0 and I had a few talks about this this sprint. His original approach was interesting, and had other benefits, but much more advanced than what the scope of this issue requires. There was supposed to be a PR with a simple fix however I do not see it in the PR list.

The simple fix affects some unit tests like comparing two outputs, so I'm now fixing them. After that, I will create a PR. This is one example affected form the fix.

nodes.enumerate.each!((idx, node) =>
        retryFor(node.getQuorumConfig() == quorums_2[idx], 5.seconds,
            format("Node %s has quorum config %s. Expected: %s",
                idx, node.getQuorumConfig(), quorums_2[idx])));

[AndrejMitrovic]

I think in the future we may want to simplify those tests and replace the hardcoded checks with something that verifies there is sufficient quorum intersection and that the quorums have been shuffled (without checking exactly their layout).

Otherwise, the tests are currently too sensitive to changes in the code.

[linked0]

Yes, right. Chris also has a similar problem in his task.
We could make a GitHub issue for that.

[AndrejMitrovic]

I just did: #1264

[Geod24]

Following live discussions:

1. This use case is likely rare (and annoying). We should have a basic safeguard against it, but not necessarily support it;
2. Safeguard #1: Add the UTXO hash to pre-images to avoid the same pre-images being propagated;
3. Safeguard #2: Add a warn logging statement when an Enrollment is encountered that is to start before our enrollment runs out;

How does it sound @linked0 ?

[linked0]

How does it sound @linked0 ?

That sounds good and simple.
Later we should keep a node from enrolling at the same height in different machines because two pre-image cycles would have the same index, which would make some critical problems when a machine starts clean. What I mean is that the pre-image cycle of one enrollment might have a different index than it is created. All the enrollments should have a different cycle index.

[Geod24]

@AndrejMitrovic : That was the discussion.

[Geod24]

Discussed, explored, and we will disallow it.