Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 6 additions & 2 deletions crates/precompiles/src/validator_config_v2/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,12 @@ mod tests {
tempo_contracts::precompiles::VALIDATOR_CONFIG_V2_ADDRESS.as_slice(),
);
msg_data.extend_from_slice(validator_addr.as_slice());
msg_data.extend_from_slice(b"192.168.1.1:8000");
msg_data.extend_from_slice(b"192.168.1.1");
let ingress = b"192.168.1.1:8000";
msg_data.push(ingress.len() as u8);
msg_data.extend_from_slice(ingress);
let egress = b"192.168.1.1";
msg_data.push(egress.len() as u8);
msg_data.extend_from_slice(egress);
let message = alloy::primitives::keccak256(&msg_data);

// Sign with namespace
Expand Down
80 changes: 65 additions & 15 deletions crates/precompiles/src/validator_config_v2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ impl ValidatorConfigV2 {
///
/// **FORMAT**:
/// - Namespace: [`VALIDATOR_NS_ADD`] or [`VALIDATOR_NS_ROTATE`]
/// - Message: `keccak256(abi.encodePacked(chainId, contractAddr, validatorAddr, ingress, egress))`
/// - Message: `keccak256(chainId || contractAddr || validatorAddr || uint8(ingress.len) || ingress || uint8(egress.len) || egress))`
fn verify_validator_signature(
&self,
namespace: &[u8],
Expand All @@ -353,7 +353,9 @@ impl ValidatorConfigV2 {
hasher.update(self.storage.chain_id().to_be_bytes());
hasher.update(VALIDATOR_CONFIG_V2_ADDRESS.as_slice());
hasher.update(validator_address.as_slice());
hasher.update([ingress.len() as u8]);
hasher.update(ingress.as_bytes());
hasher.update([egress.len() as u8]);
hasher.update(egress.as_bytes());
let message = hasher.finalize();

Expand Down Expand Up @@ -579,6 +581,14 @@ impl ValidatorConfigV2 {
self.require_new_address(v1_val.validatorAddress)?;
self.require_new_pubkey(v1_val.publicKey)?;

let deactivated_at_height = if v1_val.active {
PublicKey::decode(v1_val.publicKey.as_slice())
.map_err(|_| ValidatorConfigV2Error::invalid_public_key())?;
0
} else {
block_height
};

let egress = v1_val
.outboundAddress
.parse::<std::net::SocketAddr>()
Expand All @@ -587,8 +597,6 @@ impl ValidatorConfigV2 {

let ingress_hash = self.require_unique_ingress_ip(&v1_val.inboundAddress)?;

let deactivated_at_height = if v1_val.active { 0 } else { block_height };

self.append_validator(
v1_val.validatorAddress,
v1_val.publicKey,
Expand Down Expand Up @@ -657,7 +665,9 @@ mod tests {
data.extend_from_slice(&1u64.to_be_bytes());
data.extend_from_slice(VALIDATOR_CONFIG_V2_ADDRESS.as_slice());
data.extend_from_slice(validator_address.as_slice());
data.push(ingress.len() as u8);
data.extend_from_slice(ingress.as_bytes());
data.push(egress.len() as u8);
data.extend_from_slice(egress.as_bytes());
let message = keccak256(&data);

Expand Down Expand Up @@ -691,6 +701,11 @@ mod tests {
}
}

fn valid_pubkey(seed: u64) -> FixedBytes<32> {
let pk = PrivateKey::from_seed(seed);
FixedBytes::from_slice(&pk.public_key().encode())
}

/// Helper to make a complete add call with generated keys
fn make_valid_add_call(
addr: Address,
Expand Down Expand Up @@ -1353,7 +1368,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: v1_addr,
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand All @@ -1364,7 +1379,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: v2_addr,
publicKey: FixedBytes::<32>::from([0x22; 32]),
publicKey: valid_pubkey(2),
active: false,
inboundAddress: "192.168.1.2:8000".to_string(),
outboundAddress: "192.168.1.2:9000".to_string(),
Expand All @@ -1381,7 +1396,7 @@ mod tests {
assert_eq!(v2.validator_count()?, 1);
let migrated = v2.validator_by_index(0)?;
assert_eq!(migrated.validatorAddress, v1_addr);
assert_eq!(migrated.publicKey, FixedBytes::<32>::from([0x11; 32]));
assert_eq!(migrated.publicKey, valid_pubkey(1));
assert_eq!(migrated.deactivatedAtHeight, 0);

// Migrate second validator
Expand Down Expand Up @@ -1427,7 +1442,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: v1_addr,
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand Down Expand Up @@ -1480,7 +1495,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand All @@ -1491,7 +1506,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x22; 32]),
publicKey: valid_pubkey(2),
active: true,
inboundAddress: "192.168.1.2:8000".to_string(),
outboundAddress: "192.168.1.2:9000".to_string(),
Expand Down Expand Up @@ -1527,7 +1542,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand All @@ -1538,7 +1553,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x22; 32]),
publicKey: valid_pubkey(2),
active: true,
inboundAddress: "192.168.1.2:8000".to_string(),
outboundAddress: "192.168.1.2:9000".to_string(),
Expand All @@ -1563,6 +1578,41 @@ mod tests {
})
}

#[test]
fn test_migration_rejects_invalid_ed25519_pubkey() -> eyre::Result<()> {
let mut storage = HashMapStorageProvider::new(1);
let owner = Address::random();

StorageCtx::enter(&mut storage, || {
let mut v1 = v1();
v1.initialize(owner)?;

// [0x02; 32] is not a valid Ed25519 curve point but is non-zero so V1 accepts it.
let bad_key = [0x02u8; 32];
v1.add_validator(
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from(bad_key),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
},
)?;

let mut v2 = ValidatorConfigV2::new();
v2.storage.set_block_number(100);
let result =
v2.migrate_validator(owner, IValidatorConfigV2::migrateValidatorCall { idx: 0 });
assert_eq!(
result,
Err(ValidatorConfigV2Error::invalid_public_key().into())
);

Ok(())
})
}

#[test]
fn test_add_validator_reuses_deactivated_address() -> eyre::Result<()> {
let mut storage = HashMapStorageProvider::new(1);
Expand Down Expand Up @@ -1724,7 +1774,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: v1_addr,
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand Down Expand Up @@ -1768,7 +1818,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: v1_addr,
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand Down Expand Up @@ -1853,7 +1903,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x11; 32]),
publicKey: valid_pubkey(1),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.1.1:9000".to_string(),
Expand All @@ -1863,7 +1913,7 @@ mod tests {
owner,
tempo_contracts::precompiles::IValidatorConfig::addValidatorCall {
newValidatorAddress: Address::random(),
publicKey: FixedBytes::<32>::from([0x22; 32]),
publicKey: valid_pubkey(2),
active: true,
inboundAddress: "192.168.1.1:8000".to_string(),
outboundAddress: "192.168.2.1:9000".to_string(),
Expand Down
20 changes: 18 additions & 2 deletions tips/ref-impls/src/ValidatorConfigV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,15 @@ contract ValidatorConfigV2 is IValidatorConfigV2 {
_validateAddParams(validatorAddress, publicKey, ingress, egress);

bytes32 message = keccak256(
abi.encodePacked(block.chainid, address(this), validatorAddress, ingress, egress)
abi.encodePacked(
block.chainid,
address(this),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
_verifyEd25519Signature(
bytes("TEMPO_VALIDATOR_CONFIG_V2_ADD_VALIDATOR"), publicKey, message, signature
Expand Down Expand Up @@ -153,7 +161,15 @@ contract ValidatorConfigV2 is IValidatorConfigV2 {
_validateRotateParams(publicKey, ingress, egress);

bytes32 message = keccak256(
abi.encodePacked(block.chainid, address(this), validatorAddress, ingress, egress)
abi.encodePacked(
block.chainid,
address(this),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
_verifyEd25519Signature(
bytes("TEMPO_VALIDATOR_CONFIG_V2_ROTATE_VALIDATOR"), publicKey, message, signature
Expand Down
4 changes: 2 additions & 2 deletions tips/ref-impls/src/interfaces/IValidatorConfigV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ interface IValidatorConfigV2 {

/// @notice Add a new validator (owner only)
/// @dev The signature must be an Ed25519 signature over:
/// keccak256(abi.encodePacked("TEMPO", "_VALIDATOR_CONFIG_V2_ADD_VALIDATOR", chainId, contractAddress, validatorAddress, ingress, egress))
/// keccak256(abi.encodePacked("TEMPO", "_VALIDATOR_CONFIG_V2_ADD_VALIDATOR", chainId, contractAddress, validatorAddress, uint8(bytes(ingress).length), ingress, uint8(bytes(egress).length), egress))
/// This proves the caller controls the private key corresponding to publicKey.
/// Reverts if isInitialized() returns false.
/// @param validatorAddress The address of the new validator
Expand Down Expand Up @@ -133,7 +133,7 @@ interface IValidatorConfigV2 {
/// - Egress must be parseable as <ip>
/// - The signature must prove ownership of the new public key
/// The signature must be an Ed25519 signature over:
/// keccak256(abi.encodePacked("TEMPO", "_VALIDATOR_CONFIG_V2_ROTATE_VALIDATOR", chainId, contractAddress, validatorAddress, ingress, egress))
/// keccak256(abi.encodePacked("TEMPO", "_VALIDATOR_CONFIG_V2_ROTATE_VALIDATOR", chainId, contractAddress, validatorAddress, uint8(bytes(ingress).length), ingress, uint8(bytes(egress).length), egress))
/// @param validatorAddress The address of the validator to rotate
/// @param publicKey The new validator's Ed25519 communication public key
/// @param ingress The new validator's inbound address `<ip>:<port>` for incoming connections
Expand Down
16 changes: 14 additions & 2 deletions tips/ref-impls/test/ValidatorConfigV2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,13 @@ contract ValidatorConfigV2Test is BaseTest {
{
bytes32 message = keccak256(
abi.encodePacked(
uint64(block.chainid), address(validatorConfigV2), validatorAddress, ingress, egress
uint64(block.chainid),
address(validatorConfigV2),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
// Forge's signEd25519 does simple concat(namespace, message), but the Rust
Expand All @@ -76,7 +82,13 @@ contract ValidatorConfigV2Test is BaseTest {
{
bytes32 message = keccak256(
abi.encodePacked(
uint64(block.chainid), address(validatorConfigV2), validatorAddress, ingress, egress
uint64(block.chainid),
address(validatorConfigV2),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
bytes memory ns = bytes("TEMPO_VALIDATOR_CONFIG_V2_ROTATE_VALIDATOR");
Expand Down
16 changes: 14 additions & 2 deletions tips/ref-impls/test/invariants/ValidatorConfigV2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,13 @@ contract ValidatorConfigV2InvariantTest is InvariantBaseTest {
{
bytes32 message = keccak256(
abi.encodePacked(
uint64(block.chainid), address(validatorConfigV2), validatorAddress, ingress, egress
uint64(block.chainid),
address(validatorConfigV2),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
bytes memory ns = bytes("TEMPO_VALIDATOR_CONFIG_V2_ADD_VALIDATOR");
Expand All @@ -138,7 +144,13 @@ contract ValidatorConfigV2InvariantTest is InvariantBaseTest {
{
bytes32 message = keccak256(
abi.encodePacked(
uint64(block.chainid), address(validatorConfigV2), validatorAddress, ingress, egress
uint64(block.chainid),
address(validatorConfigV2),
validatorAddress,
uint8(bytes(ingress).length),
ingress,
uint8(bytes(egress).length),
egress
)
);
bytes memory ns = bytes("TEMPO_VALIDATOR_CONFIG_V2_ROTATE_VALIDATOR");
Expand Down
Loading
Loading