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

Feature requests for AuRa #41

Closed
varasev opened this issue Dec 3, 2018 · 24 comments
Closed

Feature requests for AuRa #41

varasev opened this issue Dec 3, 2018 · 24 comments

Comments

@varasev
Copy link

varasev commented Dec 3, 2018

Hi guys,

We'll most likely need to test the contracts with AuRa engine first.

Could we do the same things that described in issues #32, #33, and #34 for AuRa engine?

@afck
Copy link
Collaborator

afck commented Dec 3, 2018

I like the idea, because I think that in general it's good to have more than one engine that's compatible with that set of contracts.

AuRa doesn't make it easy to provide trustworthy random numbers, though. Or block timestamps! I guess for now we'd just have to trust the primary, but that way it certainly wouldn't be good for anything but testing purposes.

/cc @DrPeterVanNostrand, @mbr, @c0gent

@igorbarinov
Copy link
Member

@varasev I think we only need #33 for testing on AuRa

@varasev
Copy link
Author

varasev commented Dec 3, 2018

In the case of randao some participants should generate their random numbers and write the numbers to randao smart contract. Who will be those participants? Validators?

reportMalicious we can mocked

Note that the standard reportMalicious function requires the validator to have enough non-zero balance. I described the problem here: #32 (comment)

To remove this requirement the reportMalicious function needs to be called by system transaction (by SYSTEM_ADDRESS), but system transaction doesn't support emitting of events (I mean InitiateChange event).

So, in case of reporting malicious validators in AuRa, we also need the reportMalicious function to return the list of new validators as in case of hbbft.

Another way is to introduce support of events emitting by system transactions because currently Parity doesn't support that: https://wiki.parity.io/Validator-Set.html#limitations

@varasev
Copy link
Author

varasev commented Dec 4, 2018

  1. @afck Could we do the next thing for random in AuRa?

    Each validator node generates 20 random numbers and saves them to ValidatorSet smart contract calling its storeRandom() function. This function would XOR these numbers together (like randao does) and got resulting 20 random numbers which could be used when building a new validator set.

  2. Could we use the next function to report malicious validators instead of standard reportMalicious in AuRa?

    function reportMaliciousValidator(address _validator, uint256 _blockNumber, address _reportingValidator) public onlySystem {
        // ... if majority achieved, remove `_validator` from validator set ...
        // ... the new validator set will be available in `getValidators()` getter ...
    }

    I.e., this function could emit InitiateChange event when the function is called by SYSTEM_ADDRESS. This function would change the validator set and the new set would be available in getValidators() getter.

@afck
Copy link
Collaborator

afck commented Dec 4, 2018

I'm not sure how point 1 would work in detail: Would the other nodes store their random numbers using regular transactions, as clients? Or when it's their turn, as a primary? Either way, the last one to submit their numbers could then freely determine the result, couldn't they? So I'm wondering whether this is any better than having only the primary generate the numbers.

I agree with 2; we'll just need to figure out why SYSTEM_ADDRESS can't produce events, and fix that.

@varasev
Copy link
Author

varasev commented Dec 4, 2018

I see the next problem here: if some arbitrary participants generate the numbers, we won't have guarantees that the numbers have a uniform distribution 🤔

We need twenty 64-bit unsigned uniformly distributed random numbers (in the range from 0 to 0xffffffffffffffff).

@afck
Copy link
Collaborator

afck commented Dec 4, 2018

Exactly: With AuRa, I think it's difficult to get that kind of guarantee.
(So we'd have to use something like randao, I guess.)

@varasev
Copy link
Author

varasev commented Dec 4, 2018

Randao also uses random numbers provided by arbitrary users, so it is also affected. It seems to be a good subject to think for us.

@afck
Copy link
Collaborator

afck commented Dec 4, 2018

Yes, but in randao, the users first commit to their number without revealing it, by submitting only the hash and some amount of ETH as a pledge. This considerably ameliorates the attack: The last one to reveal their number can still refuse to do so (and pay the penalty by not getting their ETH pledge back). But at least that way, they only can choose between two outcomes (the one with and without their number), and they have an incentive to play by the rules.

In Honey Badger, it's even better: everyone also first commits to their number without revealing it, but this time not using a hash but by threshold-encrypting their number (as part of their contribution). That way, they won't even have the option later to refuse to reveal it: the other validators can collaborate to decrypt the number.

@varasev
Copy link
Author

varasev commented Dec 4, 2018

Randao is a good solution to stimulate users to reveal their seeds, but it is also trustless because they can commit ununiformly distributed numbers.

What if each validator node that creates a block would just generate and save one random number per block into the contract. Then after 20 blocks, we would be able to use the last 20 random numbers?

So, I think maybe we could trust the validator who creates a block at the moment?

@afck
Copy link
Collaborator

afck commented Dec 4, 2018

they can commit ununiformly distributed numbers

Yes, but since they are all xored together at the end, the result is uniformly distributed even if just one of the contributors was honest.
(This only works if nobody knows the others' numbers before they commit on their own—that's the reason for the hashes—and if they actually all reveal their numbers in the end—hence the pledge sum.)

Sure, we can just trust the primary validator for now. But for any production system, I think we'd need a different solution.

@varasev
Copy link
Author

varasev commented Dec 4, 2018

Another lack of randao is that one campaign for getting one random number takes some time (because the contributors need have enough time to take part in the process). So, we'll need to wait for some time to get 20 random numbers. Also, the calling contract (ValidatorSet in this case) should have some coins on its balance because randao assumes bounty.

Did I understand it correctly that you suggest calling randao's functions from validator nodes? I.e., the contributors should be validators?

@afck
Copy link
Collaborator

afck commented Dec 4, 2018

Yes, I think so. And I guess the validators can use their stakes as the bounty?
I'm not too worried about the delay: We only need them once a week anyway. If necessary, we could freeze the pools in the last hour of each staking epoch, and generate the random numbers in advance during that hour.

@varasev
Copy link
Author

varasev commented Dec 4, 2018

Ok. No, the addresses cannot use their stakes while they're validators. Also, the stakes will be made in ERC20 tokens (not in native coins). I'll think about how to use randao in our case.

@varasev
Copy link
Author

varasev commented Dec 5, 2018

One more idea for random in AuRa (refers to #41 (comment)) is to hash random number (submitted by validator's node) with a previous hash result:

function storeRandom(uint256[] _random) public onlySystem {
    require(_random.length == 20);
    for (uint256 i = 0; i < 20; i++) {
        currentRandom[i] = uint256(keccak256(currentRandom[i], _random[i]));
    }
}

So, when some validator generates 20 random numbers, they are hashed with random numbers saved previously.

In this case, it's difficult for the last validator to submit such a number that would result in predictable currentRandom[i].

@afck
Copy link
Collaborator

afck commented Dec 5, 2018

That does make it infeasible to pick an exact value for currentRandom[i], but I don't think that's enough for our purposes. The last validator to call storeRandom could easily try out lots of _random[i] values and see which new validator set that would ultimately produce, then pick one that results in a set that e.g. includes that particular validator again (or lots of their own Sybil nodes).

@varasev
Copy link
Author

varasev commented Dec 5, 2018

The problem with the last participant is also possible in case of randao :( because the current random number can be read by anyone from randao smart contract even in the time period of reveals (blockchain is open for everyone).

It seems that accumulating the random numbers in the contract before newValidatorSet is called is a bad idea.

Maybe we could pass the array of random numbers at the moment of newValidatorSet calling?

E.g., all validators' nodes know the future number of the block when newValidatorSet should be called. When this block is being created, all validators' nodes write their own numbers to the contract (and they don't know numbers of other validators) and then newValidatorSet is called (at the end of the block). Since the order of validators' transactions for each new staking epoch can be different (can be?), a validator cannot determine whether his transaction is the last in the block.

@afck
Copy link
Collaborator

afck commented Dec 5, 2018

But the problem is way less severe with randao, because by the time the numbers are revealed, everyone has already committed to their number (i.e. published its hash). So the last validator has only two options: reveal their number or not. That way they can only pick one of two results, and also only if they are willing to forfeit their pledge money.

With the above approach, the last validator could simply freely choose among any of thousands of possible values.

When this block is being created, all validators' nodes write their own numbers to the contract (and they don't know numbers of other validators)

But the primary would be the one who'd collect all the other validators' transactions, and could decide for each of them whether it should be included in the block. And worse: by the time the primary puts together the block, they do know which numbers the others have picked, so the problem would still be the same. Technically, they would be the last one to pick their number, and therefore could freely choose the result.

(Or maybe I'm misunderstanding what you're proposing: but how could, with AuRa, everyone write numbers to the contract at the same time? It's only one node who actually produces the block, isn't it?)

@varasev
Copy link
Author

varasev commented Dec 6, 2018

Yes, you're right. Seems that randao is a better solution. We need to solve what to do if not all committers reveal their seeds or if we're not collected all 20 numbers in time. I'm thinking about that...

@varasev
Copy link
Author

varasev commented Dec 7, 2018

So, I decided to create a separate Random contract: poanetwork/posdao-contracts@ab5f8d7.

This contract supports only one of two possible consensus modes at the same time: AuRa or HBBFT. The consensus mode is defined in the constructor. Initially I wanted to split the logic into two different contracts (the first for AuRa and second - for HBBFT) but later I decided to implement in one.

For HBBFT the contract contains only one function - storeRandom - which has been moved from ReportingValidatorSet.

For AuRa there're several functions needed to generate random numbers in randao manner. This is how it'll work for AuRa:

I propose to have so called collection rounds. The length of each collection round is 200 blocks. Each collection round is split into two equal phases of 100 blocks: commits phase and reveals phase.

Each phase is determined by the current block number: https://github.com/poanetwork/pos-contracts/blob/ab5f8d797fc1b3582f4feedada1fd6480e891f47/contracts/Random.sol#L118-L126

In commits phase validator's node must generate a random number, call commitHash function and pass the hash of the number. That should be done by each validator's node only once per commits phase. E.g., if we have 10 validators, the function will be called only 10 times during the commits phase.

The node can call isCommitPhase() function to know what's the current phase for the current block: https://github.com/poanetwork/pos-contracts/blob/ab5f8d797fc1b3582f4feedada1fd6480e891f47/contracts/Random.sol#L119

Then after 100 blocks (during the reveals phase) each validator's node must call revealSecret function and pass the random number that was generated by the node earlier (during the commits phase).

Once all the validators make reveals, the XORed random number is stored in public array (max length of which is 20).

Then the next collection round begins and so on.

Totally, we will have 20 random numbers refreshed permanently several times a staking epoch (which duration is 1 week).

If we take 200 blocks as the length of collection round, there will be ~600 collection rounds per staking epoch which is more than enough to generate 20 random numbers during the staking epoch even if not all validators reveal their numbers sometimes.

To implement the described mechanism we need to make Parity call the functions of Random contract from SYSTEM_ADDRESS in the manner above.

@afck
Copy link
Collaborator

afck commented Dec 8, 2018

In general, I think this works and produces actual random numbers, especially since the round lengths are long enough to allow every validator to commit and reveal. We should consider it malicious behavior to not reveal (and maybe also to not commit), to make sure the last validator can't freely decide whether to reveal or not.

But I still see a few problems with this:

Storing the random numbers on the blockchain is only useful if they are either used within the same block or if there is no way to manipulate what they are going to be used for in the meantime. More concretely:

  • If someone wanted to use this for a Roulette game, they could only use the random number in the block where it was created. Otherwise players could inspect that block, learn the next number in advance and place their bets accordingly.
  • For the PoS contracts, someone could make changes after the last random number in a staking epoch has been determined, because at that point, you can not only know but also manipulate (by adding or removing observers/pools) the outcome.

It would be nice to add a function getRandom() public returns(uint256) that returns only a single random number, and a different one for each call.
Because that, I think, would be the interface if we replaced the contract with a built-in. (And if we do that, we wouldn't need the HBBFT mode anymore, since the whole contract would be replaced.)

@varasev
Copy link
Author

varasev commented Dec 8, 2018

We should consider it malicious behavior to not reveal (and maybe also to not commit), to make sure the last validator can't freely decide whether to reveal or not.

Yes, this should be treated as misbehavior.

For the PoS contracts, someone could make changes after the last random number in a staking epoch has been determined

I think for AuRa we could deny making the changes for the last few hours of staking epoch so that the users couldn't manipulate the outcome in this case. The users will see the random numbers at the last blocks of staking epoch but won't be able to influence the outcome.

It would be nice to add a function getRandom() public returns(uint256) that returns only a single random number, and a different one for each call.
Because that, I think, would be the interface if we replaced the contract with a built-in.

I guess this is possible for HBBFT but I think we cannot do the same for AuRa because in AuRa we only have one validator producing the block, so we should trust him. So, for AuRa, I propose to implement the mechanism described above.

@afck
Copy link
Collaborator

afck commented Dec 8, 2018

Sounds good! 👍

So, for AuRa, I propose to implement the mechanism described above.

Yes, I agree.
I just mean it might be better to use a getRandom() function that would be suitable for substituting it with a built-in later, instead of a public field.

@varasev varasev closed this as completed Dec 12, 2018
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

No branches or pull requests

3 participants