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

feat: add n of m multisignature #864

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add n of m multisignature #864

wants to merge 1 commit into from

Conversation

roderik
Copy link
Member

@roderik roderik commented Feb 21, 2025

R&D for v2 - Multisig actions

Tests need to be updated!

In the dAPP

  1. Generate a unique operationId off-chain. E.g.
const operationId = ethers.utils.hexlify(ethers.utils.randomBytes(32)); 
// or some other unique approach
  1. Compute the operationHash for the mint call. For example:
const operationHash = ethers.utils.keccak256(
  ethers.utils.defaultAbiCoder.encode(["string","address","uint256"], ["MINT", to, amount])
);
  1. Build the typed data that matches MultiSigOperation(bytes32 operationId,bytes32 operationHash). Something like:
const domain = {
  name: "Equity",          // must match EIP712 domain from your contract
  version: "1",            // likewise
  chainId: <yourChainId>,
  verifyingContract: <EquityContractAddress>
};

const types = {
  MultiSigOperation: [
    { name: "operationId", type: "bytes32" },
    { name: "operationHash", type: "bytes32" },
  ]
};

const value = {
  operationId: operationId,
  operationHash: operationHash
};
  1. Ask each signer (with the SUPPLY_MANAGEMENT_ROLE in the contract) to sign this typed data:
const signature = await signer._signTypedData(domain, types, value);

For this, the signer should expose signTypedData json-rpc calls (i think they exist in the evm rc schema somehwere)

Summary by Sourcery

Implements a multi-signature access control mechanism for Equity, StableCoin, Bond, and Fund contracts, requiring multiple signatures for sensitive operations like minting, maturing, and withdrawing assets. This enhances security by preventing unauthorized actions and ensuring that critical decisions require consensus among authorized parties.

New Features:

  • Introduces a multi-signature access control mechanism for critical functions in the Equity, StableCoin, Bond, and Fund contracts, enhancing security by requiring multiple authorized parties to approve sensitive operations.
  • Adds mintWithMultisig, matureWithMultisig, withdrawUnderlyingAssetWithMultisig, withdrawExcessUnderlyingAssetsWithMultisig, and withdrawTokenWithMultisig functions to enable multi-signature authorization for minting, maturing, withdrawing assets, and withdrawing tokens.

Enhancements:

  • Replaces the standard AccessControl with a new ERC20MultiSigAccessControl contract that implements the multi-signature logic.
  • Adds a constructor parameter signatureThreshold to the Equity, StableCoin, Bond, and Fund contracts, allowing the contract creator to specify the number of signatures required for multi-signature operations.
  • Updates the EquityFactory, StableCoinFactory, BondFactory, and FundFactory contracts to include the signatureThreshold parameter in the contract creation process.

Copy link

sourcery-ai bot commented Feb 21, 2025

Reviewer's Guide by Sourcery

This pull request introduces a multi-signature access control mechanism for key functions in the Bond, Fund, StableCoin, and Equity contracts. It implements the ERC20MultiSigAccessControl contract, which extends OpenZeppelin's AccessControl and EIP712 to provide role-based multi-signature functionality. The pull request also updates the corresponding factory contracts to include the signatureThreshold parameter in the constructor and prediction logic.

Sequence diagram for mintWithMultisig function

sequenceDiagram
    participant User
    participant dApp
    participant Signer1
    participant Signer2
    participant BondContract

    User->>dApp: Initiates mintWithMultisig(to, amount)
    dApp->>dApp: Generates operationId
    dApp->>dApp: Computes operationHash (keccak256(abi.encode("MINT", to, amount)))
    dApp->>Signer1: Requests signature for (operationId, operationHash)
    activate Signer1
    Signer1-->>dApp: Signs typed data (domain, types, value)
    deactivate Signer1
    dApp->>Signer2: Requests signature for (operationId, operationHash)
    activate Signer2
    Signer2-->>dApp: Signs typed data (domain, types, value)
    deactivate Signer2
    dApp->>BondContract: mintWithMultisig(to, amount, signatures, operationId)
    activate BondContract
    BondContract->>BondContract: Checks if operationId is used
    BondContract->>BondContract: Validates signatures and role
    BondContract->>BondContract: _mint(to, amount)
    BondContract-->>dApp: Success
    deactivate BondContract
    dApp-->>User: Success
Loading

Updated class diagram for Bond contract

classDiagram
    class Bond {
        -uint256 maturityDate
        -uint256 faceValue
        -address underlyingAsset
        -bool isMatured
        -mapping(address => uint256) redeemedAmount
        +constructor(string name, string symbol, uint8 decimals, address initialOwner, string isin, uint256 _cap, uint256 _maturityDate, uint256 _faceValue, address _underlyingAsset, uint256 signatureThreshold, address forwarder)
        +mint(address to, uint256 amount)
        +mintWithMultisig(address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +mature()
        +matureWithMultisig(bytes[] calldata signatures, bytes32 operationId)
        +_mature()
        +redeem(uint256 amount)
        +withdrawUnderlyingAsset(address to, uint256 amount)
        +withdrawUnderlyingAssetWithMultisig(address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +withdrawExcessUnderlyingAssets(address to)
        +withdrawExcessUnderlyingAssetsWithMultisig(address to, bytes[] calldata signatures, bytes32 operationId)
        +withdrawableUnderlyingAmount() uint256
    }
    Bond --|> ERC20MultiSigAccessControl : inherits
    Bond --|> ERC20Burnable
    Bond --|> ERC20Pausable
    Bond --|> ERC20Permit
    Bond --|> ERC20Capped
    Bond --|> ERC20Blocklist
    Bond --|> ERC20Custodian
    Bond --|> ERC2771Context
    note for Bond "Added mintWithMultisig, matureWithMultisig, withdrawUnderlyingAssetWithMultisig, withdrawExcessUnderlyingAssetsWithMultisig functions and signatureThreshold constructor parameter"
Loading

Updated class diagram for Fund contract

classDiagram
    class Fund {
        -uint16 managementFeeBps
        -string fundClass
        -string fundCategory
        +constructor(string name, string symbol, uint8 decimals, address initialOwner, string isin, uint16 managementFeeBps_, string memory fundClass_, string memory fundCategory_, uint256 signatureThreshold, address forwarder)
        +mint(address to, uint256 amount)
        +mintWithMultisig(address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +withdrawToken(address token, address to, uint256 amount)
        +withdrawTokenWithMultisig(address token, address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +_withdrawToken(address token, address to, uint256 amount)
    }
    Fund --|> ERC20MultiSigAccessControl : inherits
    Fund --|> ERC20Burnable
    Fund --|> ERC20Pausable
    Fund --|> ERC20Permit
    Fund --|> ERC20Blocklist
    Fund --|> ERC20Custodian
    Fund --|> ERC20Votes
    Fund --|> ERC2771Context
    note for Fund "Added mintWithMultisig, withdrawTokenWithMultisig functions and signatureThreshold constructor parameter"
Loading

Updated class diagram for StableCoin contract

classDiagram
    class StableCoin {
        -CollateralProof _collateralProof
        -string isin
        +constructor(string name, string symbol, uint8 decimals, address initialOwner, string isin_, uint48 collateralLivenessSeconds, uint256 signatureThreshold, address forwarder)
        +mint(address to, uint256 amount)
        +mintWithMultisig(address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +updateCollateral(uint256 amount)
        +updateCollateralWithMultisig(uint256 amount, bytes[] calldata signatures, bytes32 operationId)
        +_updateCollateral(uint256 amount)
    }
    StableCoin --|> ERC20MultiSigAccessControl : inherits
    StableCoin --|> ERC20Burnable
    StableCoin --|> ERC20Pausable
    StableCoin --|> ERC20Permit
    StableCoin --|> ERC20Blocklist
    StableCoin --|> ERC20Collateral
    StableCoin --|> ERC20Custodian
    StableCoin --|> ERC20Votes
    StableCoin --|> ERC2771Context
    note for StableCoin "Added mintWithMultisig, updateCollateralWithMultisig functions and signatureThreshold constructor parameter"
Loading

Updated class diagram for Equity contract

classDiagram
    class Equity {
        -string isin
        -string equityClass
        -string equityCategory
        +constructor(string name, string symbol, uint8 decimals, address initialOwner, string isin_, string equityClass_, string equityCategory_, uint256 signatureThreshold, address forwarder)
        +mint(address to, uint256 amount)
        +mintWithMultisig(address to, uint256 amount, bytes[] calldata signatures, bytes32 operationId)
    }
    Equity --|> ERC20MultiSigAccessControl : inherits
    Equity --|> ERC20Burnable
    Equity --|> ERC20Pausable
    Equity --|> ERC20Permit
    Equity --|> ERC20Blocklist
    Equity --|> ERC20Custodian
    Equity --|> ERC20Votes
    Equity --|> ERC2771Context
    note for Equity "Added mintWithMultisig function and signatureThreshold constructor parameter"
Loading

Class diagram for ERC20MultiSigAccessControl contract

classDiagram
    class ERC20MultiSigAccessControl {
        -uint256 signatureThreshold
        -mapping(bytes32 => bool) _usedOperationIds
        +constructor(uint256 signatureThreshold_)
        +MULTISIG_TYPEHASH
        +withMultisig(bytes32 role, bytes[] calldata signatures, bytes32 operationId, bytes32 operationHash) modifier
        +setSignatureThreshold(uint256 newThreshold)
        +isOperationUsed(bytes32 operationId) external view returns (bool)
    }
    ERC20MultiSigAccessControl --|> AccessControl : inherits
    ERC20MultiSigAccessControl --|> EIP712 : inherits
    note for ERC20MultiSigAccessControl "Introduces multi-signature access control mechanism"
Loading

File-Level Changes

Change Details Files
Introduces a new ERC20MultiSigAccessControl contract that extends OpenZeppelin's AccessControl and EIP712 to provide role-based multi-signature functionality.
  • Adds a signatureThreshold variable to define the minimum number of signatures required for multi-sig operations.
  • Implements a withMultisig modifier to validate EIP-712 signatures from role holders.
  • Adds a setSignatureThreshold function to update the signature threshold.
  • Adds a isOperationUsed function to check if a specific operation ID has been used.
  • Introduces a MultiSigRequired error that is thrown when the signature threshold is greater than 1.
  • Implements replay protection using operationId.
kit/contracts/contracts/extensions/ERC20MultiSigAccessControl.sol
Integrates the ERC20MultiSigAccessControl contract into the Bond, Fund, StableCoin, and Equity contracts.
  • Replaces AccessControl with ERC20MultiSigAccessControl in the inheritance list.
  • Adds a signatureThreshold parameter to the constructor.
  • Adds WithMultisig variants of key functions such as mint, mature, withdrawUnderlyingAsset, withdrawExcessUnderlyingAssets, and withdrawToken.
  • Adds a check for signatureThreshold > 1 in the original functions, reverting with MultiSigRequired if true.
kit/contracts/contracts/Bond.sol
kit/contracts/contracts/Fund.sol
kit/contracts/contracts/StableCoin.sol
kit/contracts/contracts/Equity.sol
Updates the BondFactory, FundFactory, EquityFactory, and StableCoinFactory contracts to include the signatureThreshold parameter in the constructor and prediction logic.
  • Adds a signatureThreshold parameter to the create function.
  • Updates the predictAddress function to include the signatureThreshold parameter in the address prediction calculation.
  • Passes the signatureThreshold parameter to the constructor of the deployed contract.
kit/contracts/contracts/FundFactory.sol
kit/contracts/contracts/BondFactory.sol
kit/contracts/contracts/EquityFactory.sol
kit/contracts/contracts/StableCoinFactory.sol

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions bot added the feat label Feb 21, 2025
@roderik roderik marked this pull request as draft February 21, 2025 06:04
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @roderik - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding a function to query the signatureThreshold during contract creation in the factory contracts.
  • The operationHash in withMultisig should include the contract address to prevent cross-contract replay attacks.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Review instructions: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@roderik roderik requested a review from daanporon February 21, 2025 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant