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 request for AuRa: call reportMalicious with zero gas price #50

Closed
varasev opened this issue Dec 12, 2018 · 7 comments
Closed

Feature request for AuRa: call reportMalicious with zero gas price #50

varasev opened this issue Dec 12, 2018 · 7 comments
Assignees

Comments

@varasev
Copy link

varasev commented Dec 12, 2018

Every validator in AuRa calls reportMalicious function when he sees misbehavior of another validator: https://wiki.parity.io/Validator-Set.html#reporting-contract

We need to make its calling with zero gas price to let the validators report with zero wallet's balance.

The strikethrough text below is no longer relevant.

The reportMaliciousValidator function must be called by SYSTEM_ADDRESS in AuRa instead of standard reportMalicious function (described in https://wiki.parity.io/Validator-Set.html).

The standard reportMalicious function looks like this:

function reportMalicious(address _validator, uint256 _blockNumber, bytes _proof) public

So, what we should do with it:

- call reportMaliciousValidator instead of reportMalicious and do that calling from SYSTEM_ADDRESS (instead of validator's address).
- pass the _message and _signature parameters instead of _validator, _blockNumber, _proof:

function reportMaliciousValidator(bytes _message, bytes _signature) public onlySystem {...}

The _message parameter is a bytes sequence which contains the address of malicious validator (see _validator param for the standard reportMalicious function) and block number (see _blockNumber param for the standard reportMalicious function) on which the validator misbehaved. Inside the contract this parameter is parsed as follows: https://github.com/poanetwork/pos-contracts/blob/1240a2e6ab098f18c868324fb2c22454417807d5/contracts/ValidatorSetAuRa.sol#L53-L56

So, the _message param has a length of 52 bytes: 20 bytes for malicious validator address and 32 bytes for block number.

The _message should be signed by validator's private key, and the resulting signature should be passed through the _signature param.

The signature is needed to prevent the possibility of passing arbitrary validator's address and is used to recover validator's address using Solidity's ecrecover function: https://github.com/poanetwork/pos-contracts/blob/1240a2e6ab098f18c868324fb2c22454417807d5/contracts/ValidatorSetAuRa.sol#L138

I don't know how the signing code could be implemented in Rust, but this should work as web3's sign function works: web3.eth.accounts.sign. Some examples are available here: https://ethereum.stackexchange.com/questions/1777/workflow-on-signing-a-string-with-private-key-followed-by-signature-verificatio

@vkomenda
Copy link

I think we should use ethkey::sign as the signing function rather than EngineSigner's one to sign the message as it was done here:

self.validators.report_malicious(header.author(), set_number, header.number(), &mut |hash| self.sign(hash).expect("bug"));

The ecrecover specification calls for an ECDSA signature with components r, s and v. This is what ethkey::sign produces.

@DemiMarie
Copy link

@vkomenda EngineSigner::sign produces an ethkey::Signature which is also the return type (on success) of ethkey::sign. So we should be fine.

@vkomenda
Copy link

vkomenda commented Jan 3, 2019

I see. That's convenient for getting the account private key. I improved typing a bit:


We can return the Error cleanly. Also there is no need to make the function closure argument mutable.

@DemiMarie
Copy link

@vkomenda Thank you.

@vkomenda vkomenda self-assigned this Jan 7, 2019
@afck
Copy link
Collaborator

afck commented Jan 9, 2019

@vkomenda: Since we're going to use regular service transactions now (see #57), we will go back to calling reportMalicious. To make it a service transaction, just set the gas price to zero.

@varasev varasev changed the title Feature request for AuRa: reportMaliciousValidator Feature request for AuRa: call reportMalicious with zero gas price Jan 9, 2019
@varasev
Copy link
Author

varasev commented Jan 9, 2019

I've updated the description ☝️

@DemiMarie
Copy link

Fixed by #59.

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

4 participants