Skip to content

Commit

Permalink
fix(contracts/core): include msg sender in createValidator digest
Browse files Browse the repository at this point in the history
Additional:
- refactor to use 33 byte compressed pubkey, instead of x y coords
  • Loading branch information
kevinhalliday committed Jan 10, 2025
1 parent 6f4aa1b commit 3d2fe52
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 131 deletions.
2 changes: 1 addition & 1 deletion cli/cmd/staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func createValidator(ctx context.Context, cfg createValConfig) error {
return err
}

tx, err := contract.CreateValidator(txOpts, crypto.CompressPubkey(consPubkey))
tx, err := contract.CreateValidator0(txOpts, crypto.CompressPubkey(consPubkey))
if err != nil {
return errors.Wrap(err, "create validator")
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/bindings/admin.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/bindings/allocpredeploys.go

Large diffs are not rendered by default.

76 changes: 38 additions & 38 deletions contracts/bindings/staking.go

Large diffs are not rendered by default.

23 changes: 12 additions & 11 deletions contracts/core/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
Admin_Test:test_pause_unpause() (gas: 34028415)
Admin_Test:test_pause_unpause_bridge() (gas: 29058168)
Admin_Test:test_pause_unpause_xcall() (gas: 33982389)
Admin_Test:test_pause_unpause_xsubmit() (gas: 33982096)
Admin_Test:test_upgrade() (gas: 38063590)
AllocPredeploys_Test:test_num_allocs() (gas: 1181319931)
AllocPredeploys_Test:test_predeploys() (gas: 1181301741)
AllocPredeploys_Test:test_preinstalls() (gas: 1182018157)
AllocPredeploys_Test:test_proxies() (gas: 1408944958)
Admin_Test:test_pause_unpause() (gas: 33983688)
Admin_Test:test_pause_unpause_bridge() (gas: 29013441)
Admin_Test:test_pause_unpause_xcall() (gas: 33937662)
Admin_Test:test_pause_unpause_xsubmit() (gas: 33937369)
Admin_Test:test_upgrade() (gas: 38018863)
AllocPredeploys_Test:test_num_allocs() (gas: 1181359031)
AllocPredeploys_Test:test_predeploys() (gas: 1181295355)
AllocPredeploys_Test:test_preinstalls() (gas: 1182011771)
AllocPredeploys_Test:test_proxies() (gas: 1408938572)
FeeOracleV1_Test:test_bulkSetFeeParams() (gas: 173154)
FeeOracleV1_Test:test_feeFor() (gas: 122830)
FeeOracleV1_Test:test_setBaseGasLimit() (gas: 32375)
Expand Down Expand Up @@ -149,8 +149,9 @@ Quorum_Test:test_verify_sigsNotSorted_reverts() (gas: 282992)
Quorum_Test:test_verify_succeeds() (gas: 294059)
Slashing_Test:test_stub() (gas: 143)
Slashing_Test:test_unjail() (gas: 54734)
Staking_Test:test_createValidator() (gas: 145288)
Staking_Test:test_delegate() (gas: 98848)
Staking_Test:test_createValidator_withSignature() (gas: 363511)
Staking_Test:test_createValidator_withoutSignature() (gas: 228383)
Staking_Test:test_delegate() (gas: 102798)
Staking_Test:test_stub() (gas: 143)
Upgrade_Test:test_stub() (gas: 143)
XAppUpgradeable_Test:test_isXCall() (gas: 148924)
Expand Down
13 changes: 7 additions & 6 deletions contracts/core/script/admin/StakingPostUpgradeTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,20 @@ contract StakingPostUpgradeTest is Test {

function _testCreateValidator() internal {
bytes32 privkey = 0xf0e1605dd50ce33553290b778b0f53b2cde5e47a8794c0e7d2815e456e6da3b9;
bytes32 x = 0x3b12d750493ed6b12b390447f6dd38f587af12ed04ab8d6858e818cf0c63607c;
bytes32 y = 0x044e0321a3e57de51e95f2b230b9e4ffed2318578baab1a80652234fe0115d13;
bytes memory pubkey = Secp256k1.compressPublicKey(x, y);
bytes32 digest = staking.getValidatorPubkeyDigest(x, y);
uint256 x = 0x3b12d750493ed6b12b390447f6dd38f587af12ed04ab8d6858e818cf0c63607c;
uint256 y = 0x044e0321a3e57de51e95f2b230b9e4ffed2318578baab1a80652234fe0115d13;
bytes memory pubkey = Secp256k1.compress(x, y);
bytes memory compressed = Secp256k1.compress(x, y);
bytes32 digest = staking.getConsPubkeyDigest(validator);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(uint256(privkey), digest);
bytes memory signature = abi.encodePacked(r, s, v);
uint256 deposit = staking.MinDeposit();
vm.deal(validator, deposit);

vm.expectEmit();
emit Staking.CreateValidator(validator, pubkey, deposit);
emit Staking.CreateValidator(validator, compressed, deposit);
vm.prank(validator);
staking.createValidator{ value: deposit }(x, y, signature);
staking.createValidator{ value: deposit }(pubkey, signature);
}

function _testDelegate() internal {
Expand Down
58 changes: 32 additions & 26 deletions contracts/core/src/libraries/Secp256k1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,61 +24,67 @@ library Secp256k1 {
uint256 public constant PP = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F;

/**
* @notice Compress a public key
* @notice Returns 33 byte compressed public key for the given x, y coordinates
* @param x The x coordinate of the public key
* @param y The y coordinate of the public key
* @return compressedPubKey The compressed public key
*/
function compressPublicKey(bytes32 x, bytes32 y) internal pure returns (bytes memory) {
function compress(uint256 x, uint256 y) internal pure returns (bytes memory) {
bytes32 _y = bytes32(y);

// Set prefix based on y coordinate's parity
// 0x02 for even y, 0x03 for odd y
uint8 prefix = uint8(y[31] & 0x01) + 0x02;
uint8 prefix = uint8(_y[31] & 0x01) + 0x02;

// Concatenate prefix and x coordinate
bytes memory compressedPubKey = abi.encodePacked(bytes1(prefix), x);
return compressedPubKey;
}

/**
* @notice Verify a secp256k1 public key is on the curve
* @param x The x coordinate of the public key
* @param y The y coordinate of the public key
* @return True if the public key is valid, false otherwise
*/
function verifyPubkey(bytes32 x, bytes32 y) internal pure returns (bool) {
return EllipticCurve.isOnCurve(uint256(x), uint256(y), AA, BB, PP);
return abi.encodePacked(bytes1(prefix), x);
}

/**
* @notice Validate a compressed secp256k1 public key
* @param compressedPubKey The compressed public key to validate
* @return True if the public key is valid, false otherwise
* @notice Returns x,y coordinates of an compressed public key
* Reverts if the public key is invalid
* @param pubkey The 33 byte compressed public key
*/
function verifyPubkey(bytes calldata compressedPubKey) internal pure returns (bool) {
require(compressedPubKey.length == 33, "Staking: invalid pubkey length");
require(compressedPubKey[0] == 0x02 || compressedPubKey[0] == 0x03, "Staking: invalid pubkey prefix");
function decompress(bytes calldata pubkey) internal pure returns (uint256, uint256) {
require(pubkey.length == 33, "Secp256k1: pubkey not 33 bytes");
require(pubkey[0] == 0x02 || pubkey[0] == 0x03, "Secp256k1: invalid pubkey prefix");

// Extract x coordinate
uint256 x;
assembly {
let ptr := add(compressedPubKey.offset, 1)
let ptr := add(pubkey.offset, 1)
x := calldataload(ptr)
}

// Derive y coordinate
uint256 y = EllipticCurve.deriveY(uint8(compressedPubKey[0]), x, AA, BB, PP);
uint256 y = EllipticCurve.deriveY(uint8(pubkey[0]), x, AA, BB, PP);

require(onCurve(x, y), "Secp256k1: pubkey not on curve");

// Verify the derived point lies on the curve
return (x, y);
}

/**
* @notice Return true if x, y are on the secp256k1 curve
*/
function onCurve(uint256 x, uint256 y) internal pure returns (bool) {
return EllipticCurve.isOnCurve(x, y, AA, BB, PP);
}

/**
* @notice Revert if the public key is invalid
* @param pubkey The compressed public key to validate
*/
function verify(bytes calldata pubkey) internal pure {
decompress(pubkey);
}

/**
* @notice Convert public key coordinates to an Ethereum address
* @param x The x coordinate of the public key
* @param y The y coordinate of the public key
* @return The Ethereum address corresponding to the public key
*/
function pubkeyToAddress(bytes32 x, bytes32 y) internal pure returns (address) {
function pubkeyToAddress(uint256 x, uint256 y) internal pure returns (address) {
bytes memory pubKey = new bytes(64);
assembly {
// Store x and y coordinates
Expand Down
45 changes: 22 additions & 23 deletions contracts/core/src/octane/Staking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -110,41 +110,40 @@ contract Staking is OwnableUpgradeable, EIP712Upgradeable {

/**
* @notice Create a new validator
* @param pubkey The validators consensus public key. 33 bytes compressed secp256k1 public key
* @param pubkey The validators consensus public key. 33 byte compressed secp256k1 public key
* @dev Proxies x/staking.MsgCreateValidator
* @dev NOTE: This function needs to be removed once Go codebase is migrated to the new functions below
*/
function createValidator(bytes calldata pubkey) external payable {
require(!isAllowlistEnabled || isAllowedValidator[msg.sender], "Staking: not allowed");
require(msg.value >= MinDeposit, "Staking: insufficient deposit");
require(Secp256k1.verifyPubkey(pubkey), "Staking: invalid pubkey");
Secp256k1.verify(pubkey);

emit CreateValidator(msg.sender, pubkey, msg.value);
}

/**
* @param x The x coordinate of the validators consensus public key
* @param y The y coordinate of the validators consensus public key
* @return Digest hash to be signed by the validators public key
* @param validator The validator address (msg.sender in createValidator)
* @return Digest hash to be signed by the validators consesnsus public key,
* authorizing the validator to use that consensus key.
*/
function getValidatorPubkeyDigest(bytes32 x, bytes32 y) external view returns (bytes32) {
return _hashTypedDataV4(keccak256(abi.encode(_EIP712_TYPEHASH, x, y)));
function getConsPubkeyDigest(address validator) public view returns (bytes32) {
return _hashTypedDataV4(keccak256(abi.encode(_EIP712_TYPEHASH, validator)));
}

/**
* @notice Create a new validator
* @param x The x coordinate of the validators consensus public key
* @param y The y coordinate of the validators consensus public key
* @param signature The signature of the validators consensus public key
* @param pubkey The validators consensus public key. 33 byte compressed secp256k1 public key
* @param signature Signature of getConsPubkeyDigest(validator) by pubkey
* @dev Proxies x/staking.MsgCreateValidator
*/
function createValidator(bytes32 x, bytes32 y, bytes calldata signature) external payable {
function createValidator(bytes calldata pubkey, bytes calldata signature) external payable {
require(!isAllowlistEnabled || isAllowedValidator[msg.sender], "Staking: not allowed");
require(msg.value >= MinDeposit, "Staking: insufficient deposit");
require(Secp256k1.verifyPubkey(x, y), "Staking: invalid pubkey");
require(_verifySignature(x, y, signature), "Staking: invalid signature");

bytes memory pubkey = Secp256k1.compressPublicKey(x, y);
(uint256 x, uint256 y) = Secp256k1.decompress(pubkey);
_verifySignature(x, y, msg.sender, signature);

emit CreateValidator(msg.sender, pubkey, msg.value);
}

Expand Down Expand Up @@ -211,16 +210,16 @@ contract Staking is OwnableUpgradeable, EIP712Upgradeable {
//////////////////////////////////////////////////////////////////////////////

/**
* @notice Verify a signature matches a secp256k1 public key
* @param x The x coordinate of the validators consensus public key
* @param y The y coordinate of the validators consensus public key
* @param signature The signature of the validators consensus public key
* @notice Verifies signature is getConsPubkeyDigest(validator) signed by x,y pubkey
* Revokes the signature if invalid
* @param x The x coordinate of the validators consensus public key
* @param y The y coordinate of the validators consensus public key
* @param validator The validator address signed by the consensus key
* @param signature The signature of the validator adddress, by the consensus public key
*/
function _verifySignature(bytes32 x, bytes32 y, bytes calldata signature) internal view returns (bool) {
bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(_EIP712_TYPEHASH, x, y)));
(address recovered,,) = ECDSA.tryRecover(digest, signature);
address pubKeyAddress = Secp256k1.pubkeyToAddress(x, y);
return recovered == pubKeyAddress;
function _verifySignature(uint256 x, uint256 y, address validator, bytes calldata signature) internal view {
(address recovered,,) = ECDSA.tryRecover(getConsPubkeyDigest(validator), signature);
require(recovered == Secp256k1.pubkeyToAddress(x, y), "Staking: invalid signature");
}

/**
Expand Down
Loading

0 comments on commit 3d2fe52

Please sign in to comment.