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

Validator Signatures #6

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions HIP-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
| hip | title | status | author | created |
| --- | -------------------- | ------ | ------ | ---------- |
| 2 | Validator signatures | Draft | asaj | 2022-12-20 |

### **Brief Summary / Abstract**

Defines the specification for Hyperlane validator signatures.

Hyperlane validators sign messages that act as attestations of the form:

> "On chain _d_, the mailbox contract at address _m_ had a messages tree with merkle root _r_ and message count _i_"
asaj marked this conversation as resolved.
Show resolved Hide resolved

### **Motivation**

Hyperlane validators provide the foundation for two core interchain security modules types: Multisig, and Optimistic.

This HIP defines a standard for validator signatures so that they can be shared across different security module instances.

### **Tech Spec**

A Hyperlane validator should sign the following, in compliance with [EIP-191](https://eips.ethereum.org/EIPS/eip-191).

```
/**
* @notice Returns the digest that Hyperlane validators should sign
* @param _domain The origin domain of the Mailbox being validated
* @param _mailbox The address of the Mailbox being validated, as bytes32
* @param _root The merkle root that the validator is attesting to
* @param _index The message count that the validator is attesting to
* @return The digest to EIP-191 sign
*/
function getDigestToSign(
uint32 _domain,
bytes32 _mailbox,
bytes32 _root,
uint32 _index
)
external
pure
returns (bytes32)
{

bytes32 _domainHash = keccak256(
abi.encodePacked(_domain, _mailbox, "HYPERLANE")
);
return keccak256(abi.encodePacked(_domainHash, _root, _index));
asaj marked this conversation as resolved.
Show resolved Hide resolved
}
```

A `ValidatorSignatureVerifier` contract implementing the following interface should be deployed to each chain supporting Hyperlane. This contract must implement the following interface and emit the `ValidatorSignature` event when verifying a signature.

Verifying signatures via `ValidatorSignatureVerifier` allows watchers to more easily monitor for fraudulent validator signatures.

```
interface IValidatorSignatureVerifier {
/**
* @notice Emitted when a validator signature is verified
* @dev Used by watchtowers to detect fraudulent validators
* @param domain The origin domain of the Mailbox being validated
* @param mailbox The address of the Mailbox being validated, as bytes32
* @param root The merkle root that the validator is attesting to
* @param index The message count that the validator is attesting to
* @param signature The 65-byte ECDSA validator signature
*/
event ValidatorSignature(
uint32 domain,
bytes32 mailbox,
bytes32 root,
uint32 index
bytes signature
);
Comment on lines +57 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems expensive to emit this for every signature verification

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of redundant event info every message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good call, it's about 600 gas to emit the domain/mailbox/root/index, plus another couple hundred for the event itself.

Maybe this would be better? Wdyt?

event ValidatorSignatures(
        uint32 domain,
        bytes32 mailbox,
        bytes32 root,
        uint32 index
        bytes[] signatures
    );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo domain and mailbox are unnecessary as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't the root implicitly commit to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not. Even if did, as a watcher, how would you recover the mailbox/origin domain in order to construct your fraud proof?


/**
* @param _domain The origin domain of the Mailbox being validated
* @param _mailbox The address of the Mailbox being validated, as bytes32
* @param _root The merkle root that the validator is attesting to
* @param _index The message count that the validator is attesting to
* @param _signature The 65-byte ECDSA validator signature
* @return The address of the validator that signed
*/
function recoverValidatorFromSignature(
asaj marked this conversation as resolved.
Show resolved Hide resolved
uint32 _domain,
bytes32 _mailbox,
bytes32 _root,
uint32 _index,
bytes calldata _signature
) external returns (address);
}
```

### **Rationale**

#### Inclusion of origin mailbox address

Including the origin mailbox address in the validator signature gives it clearer semantic meaning that can be verified:

> > "On chain _d_, the mailbox contract at address _m_ had a messages tree with merkle root _r_ and message count _i_"

The downside is that care will need to be taken in order to ensure that malicious validators cannot manipulate the mailbox address in their signature in order to avoid the consequences of fraud.

Take the following psuedo-code for a slashing contract:

```
function slash(
uint32 _domain,
bytes32 _mailbox,
bytes32 _root,
uint32 _index,
bytes calldata _signature
) external returns (address) {
address _validator = signatureVerifier.recoverValidatorFromSignature(
_domain,
_mailbox,
_root,
_index,
_signature
);
require(IMailbox(_mailbox).rootAt(_index) != _root);
_slash(_validator);
}
```

Validators could avoid the consequences of signing a fraudulent checkpoint by deploying a contract that implements the `IMailbox` interface but allows them to push arbitrary messages to it. They could then sign a checkpoint for this contract and attempt to use that to deliver fraudulent messages on some destination chain.

There are two potential mitigations:

1. Encode remote mailbox addresses in ISMs
This would ensure that ISMs only accept messages from mailboxes that the ISM deployer knows to be a correct implementation of `Mailbox`. This ensures that `slash()` will always succeed if the validator attempts to attest to a root containing fraudulent messages.

2. Encode supported mailboxes in the slashing contract
This would mean that signing a checkpoint for an unsupported mailbox would be considered a slashable offense. This could be done either by hardcoding addresses in storage, or hardcoding supported mailbox implementation bytecode, presuming the contents of mailbox storage do not affect the trust assumptions of outbound messages (they do not as of 2022-12-21).

#### Encouraging verification through ValidatorSignatureVerifier

While less gas efficient, this ensures that there is a single contract that watchers can monitor for fraudulent validator signatures.

Fraudulent validator signatures may be used to slash the fraudulent validator, or as a trigger to disconnect an optimistic ISM.

Encouraging a single implementation also reduces the risk that validator-signature-based ISM implementers introduce bugs in signature verification.

### **Backwards Compatibility**

This format is not backwards compatible with V1, which is actually a good thing as it means validator signatures cannot be re-used.

The default MultisigIsm in the V2 pre-release does not use a `ValidatorSignatureVerifier`. This can be corrected in future ISM deployements.

### **Security Considerations**

A bug in `ValidatorSignatureVerifier` would risk compromising all validator-signature-based ISMs.