Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Commit

Permalink
fix: registry sig reusage (#28)
Browse files Browse the repository at this point in the history 
* fix registry sig reusage

* fea: backwards participant presence check

* add & change comments

* fix clearing migration example

* get inter payload return if participant is absent

* tests updated

* ref deploy helpers

* rename Interaction to Identity

* use contract field instead of param in constructor

* use getIdentityPayload in code more

Co-authored-by: nksazonov <nsazonov@openware.com>
  • Loading branch information
@nksazonov
nksazonov and nksazonov authored Oct 24, 2022
1 parent bdbc1bb commit 480102c
Show file tree
Hide file tree
Showing 8 changed files with 487 additions and 119 deletions.
154 changes: 122 additions & 32 deletions contracts/clearing/YellowClearingBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ abstract contract YellowClearingBase is AccessControl {

// Participant data
struct ParticipantData {
uint64 registrationTime;
ParticipantStatus status;
uint64 nonce;
uint64 registrationTime;
}

// Participant identity payload
struct IdentityPayload {
YellowClearingBase YellowClearing;
address participant;
uint64 nonce;
}

// Roles
Expand All @@ -43,7 +51,8 @@ abstract contract YellowClearingBase is AccessControl {
// Participant data mapping
mapping(address => ParticipantData) internal _participantData;

// Next implementation
// Prev and next implementations
YellowClearingBase private immutable _prevImplementation;
YellowClearingBase private _nextImplementation;

// Address of this contract
Expand All @@ -57,8 +66,10 @@ abstract contract YellowClearingBase is AccessControl {
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(REGISTRY_MAINTAINER_ROLE, msg.sender);

if (address(previousImplementation) != address(0)) {
_grantRole(PREVIOUS_IMPLEMENTATION_ROLE, address(previousImplementation));
_prevImplementation = previousImplementation;

if (address(_prevImplementation) != address(0)) {
_grantRole(PREVIOUS_IMPLEMENTATION_ROLE, address(_prevImplementation));
}
}

Expand Down Expand Up @@ -114,16 +125,39 @@ abstract contract YellowClearingBase is AccessControl {
}

/**
* @notice Recursively check that participant is not present in this registry and any subsequent.
* @dev Recursively check that participant is not present in this registry and all subsequent.
* @notice Recursively check that participant is not present in this registry and all previous ones.
* @dev Recursively check that participant is not present in this registry and all previous ones.
* @param participant Address of participant to check.
*/
function requireParticipantNotPresentBackwards(address participant) public view {
if (address(_prevImplementation) != address(0)) {
_prevImplementation.requireParticipantNotPresentBackwards(participant);
}

_requireParticipantNotPresent(participant);
}

/**
* @notice Recursively check that participant is not present in this registry and all subsequent ones.
* @dev Recursively check that participant is not present in this registry and all subsequent ones.
* @param participant Address of participant to check.
*/
function requireParticipantNotPresent(address participant) public view {
function requireParticipantNotPresentForwards(address participant) public view {
if (address(_nextImplementation) != address(0)) {
_nextImplementation.requireParticipantNotPresent(participant);
_nextImplementation.requireParticipantNotPresentForwards(participant);
}

require(!hasParticipant(participant), 'Participant already registered');
_requireParticipantNotPresent(participant);
}

/**
* @notice Recursively check that participant is not present in this registry and all previous and subsequent ones.
* @dev Recursively check that participant is not present in this registry and all previous and subsequent ones.
* @param participant Address of participant to check.
*/
function requireParticipantNotPresentRecursive(address participant) public view {
requireParticipantNotPresentBackwards(participant);
requireParticipantNotPresentForwards(participant);
}

/**
Expand All @@ -137,29 +171,58 @@ abstract contract YellowClearingBase is AccessControl {
view
returns (ParticipantData memory)
{
require(hasParticipant(participant), 'Participant does not exist');
_requireParticipantPresent(participant);

return _participantData[participant];
}

/**
* @notice Return identity payload structure for a supplied participant. Used to ease interaction with this contract.
* @dev Return identity payload structure for a supplied participant. Used to ease interaction with this contract.
* @param participant Address of participant to get identity payload for.
* @return IdentityPayload Identity payload structure for a supplied participant.
*/
function getIdentityPayload(address participant) public view returns (IdentityPayload memory) {
uint64 nonce;

if (!hasParticipant(participant)) {
nonce = 0;
} else {
nonce = _participantData[participant].nonce + 1;
}

return
IdentityPayload({
YellowClearing: YellowClearingBase(_self),
participant: participant,
nonce: nonce
});
}

// ======================
// participant changes
// ======================

/**
* @notice Register participant by adding it to the registry with Pending status. Emit `ParticipantRegistered` event.
* @dev Participant must not be present in this or any subsequent implementations.
* @dev Participant must not be present in this or any previous or subsequent implementations.
* @param participant Virtual (no address, only public key exist) address of participant to add.
* @param signature Participant virtual address signer by this same participant.
* @param signature Participant identity payload signed by this same participant.
*/
function registerParticipant(address participant, bytes calldata signature) external {
requireParticipantNotPresent(participant);
requireParticipantNotPresentRecursive(participant);

IdentityPayload memory identityPayload = getIdentityPayload(participant);

require(_recoverAddressSigner(participant, signature) == participant, 'Invalid signer');
require(
_recoverIdentitySigner(identityPayload, signature) == participant,
'Invalid signer'
);

_participantData[participant] = ParticipantData({
registrationTime: uint64(block.timestamp),
status: ParticipantStatus.Pending
status: ParticipantStatus.Pending,
nonce: identityPayload.nonce,
registrationTime: uint64(block.timestamp)
});

emit ParticipantRegistered(participant);
Expand All @@ -172,7 +235,7 @@ abstract contract YellowClearingBase is AccessControl {
* @param participant Address of participant to validate.
*/
function validateParticipant(address participant) external onlyRole(REGISTRY_VALIDATOR_ROLE) {
require(hasParticipant(participant), 'Participant does not exist');
_requireParticipantPresent(participant);
require(
_participantData[participant].status == ParticipantStatus.Pending,
'Invalid status'
Expand All @@ -191,7 +254,7 @@ abstract contract YellowClearingBase is AccessControl {
* @param participant Address of participant to suspend.
*/
function suspendParticipant(address participant) external onlyRole(AUDITOR_ROLE) {
require(hasParticipant(participant), 'Participant does not exist');
_requireParticipantPresent(participant);

ParticipantStatus status = _participantData[participant].status;
require(
Expand All @@ -213,7 +276,7 @@ abstract contract YellowClearingBase is AccessControl {
* @param participant Address of participant to reinstate.
*/
function reinstateParticipant(address participant) external onlyRole(AUDITOR_ROLE) {
require(hasParticipant(participant), 'Participant does not exist');
_requireParticipantPresent(participant);
require(
_participantData[participant].status == ParticipantStatus.Suspended,
'Invalid status'
Expand Down Expand Up @@ -255,26 +318,36 @@ abstract contract YellowClearingBase is AccessControl {
* @notice Migrate participant to the newest implementation present in upgrades chain. Emit `ParticipantMigratedFrom` and `ParticipantMigratedTo` events.
* @dev NextImplementation must have been set. Participant must not have been migrated.
* @param participant Address of participant to migrate.
* @param signature Participant address signed by that participant.
* @param signature Participant identity payload signed by that participant.
*/
function migrateParticipant(address participant, bytes calldata signature) external {
require(address(_nextImplementation) != address(0), 'Next implementation is not set');

require(hasParticipant(participant), 'Participant does not exist');
_requireParticipantPresent(participant);

require(_recoverAddressSigner(participant, signature) == participant, 'Invalid signer');
IdentityPayload memory identityPayload = getIdentityPayload(participant);

require(
_recoverIdentitySigner(identityPayload, signature) == participant,
'Invalid signer'
);

// Get previous participant data
ParticipantData memory currentData = _participantData[participant];
require(currentData.status != ParticipantStatus.Migrated, 'Participant already migrated');

// Update data to resemble migration
ParticipantData memory updatedData = currentData;
updatedData.nonce = identityPayload.nonce;

// Migrate data, emit ParticipantMigratedTo
_nextImplementation.migrateParticipantData(participant, currentData);
_nextImplementation.migrateParticipantData(participant, updatedData);

// Mark participant as migrated on this implementation
_participantData[participant] = ParticipantData({
registrationTime: currentData.registrationTime,
status: ParticipantStatus.Migrated
status: ParticipantStatus.Migrated,
nonce: updatedData.nonce,
registrationTime: updatedData.registrationTime
});

// Emit event
Expand Down Expand Up @@ -304,20 +377,37 @@ abstract contract YellowClearingBase is AccessControl {
// internal functions
// ======================

// Recover signer from `signature` provided `_address` is signed.
/**
* @notice Recover signer of the address.
* @dev Recover signer of the address.
* @param _address Address to be signed.
* @param signature Signed address.
* @notice Require participant it present in this registry.
* @dev Require participant it present in this registry.
* @param participant Address of participant to check.
*/
function _requireParticipantPresent(address participant) internal view {
require(hasParticipant(participant), 'Participant does not exist');
}

/**
* @notice Require participant it not present in this registry.
* @dev Require participant it not present in this registry.
* @param participant Address of participant to check.
*/
function _requireParticipantNotPresent(address participant) internal view {
require(!hasParticipant(participant), 'Participant already exist');
}

/**
* @notice Recover signer of identity payload.
* @dev Recover signer of identity payload.
* @param identityPayload Identity payload that has been signed.
* @param signature Signed identity payload.
* @return address Address of the signer.
*/
function _recoverAddressSigner(address _address, bytes memory signature)
function _recoverIdentitySigner(IdentityPayload memory identityPayload, bytes memory signature)
internal
pure
returns (address)
{
return keccak256(abi.encode(_address)).toEthSignedMessageHash().recover(signature);
return keccak256(abi.encode(identityPayload)).toEthSignedMessageHash().recover(signature);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/clearing/test/TESTYellowClearingV3.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ contract TESTYellowClearingV3 is YellowClearingBase {
internal
override
{
ParticipantData memory migratedData = ParticipantData(42, data.status);
ParticipantData memory migratedData = ParticipantData(data.status, data.nonce, 42);

_participantData[participant] = migratedData;
}
Expand Down
4 changes: 2 additions & 2 deletions src/revert-reasons.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {utils} from 'ethers';
import { utils } from 'ethers';

const hexify = utils.hexlify;

Expand Down Expand Up @@ -29,7 +29,7 @@ export const INVALID_CHAIN_ID = 'Invalid chain id';
// network registry
export const NEXT_IMPL_ALREADY_SET = 'Next implementation already set';
export const PREV_IMPL_ROLE_REQUIRED = 'Previous implementation role is required';
export const PARTICIPANT_ALREADY_REGISTERED = 'Participant already registered';
export const PARTICIPANT_ALREADY_EXIST = 'Participant already exist';
export const NO_PARTICIPANT = 'Participant does not exist';
export const INVALID_SIGNER = 'Invalid signer';
export const INVALID_STATUS = 'Invalid status';
Expand Down
Loading

0 comments on commit 480102c

Please sign in to comment.