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: improve multisig utility and usability #5027

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

s8sato
Copy link
Contributor

@s8sato s8sato commented Sep 3, 2024

BREAKING CHANGES:

  • (api-changes) CanRegisterAnyTrigger CanUnregisterAnyTrigger permissions for system authority
  • (api-changes) GenesisWasmTrigger in RawGenesisTransaction for genesis.json readability
  • (api-changes) Multisig*Args for multi-signature operations
  • (config-changes) genesis.json assumes wasm_triggers[*].action.executable is prebuilt under wasm_dir
  • (config-changes) docker-compose service generates genesis.json by oneself instead of reading bind-mounted one

Major commits:

  • feat: support multisig recursion
  • feat: introduce multisig quorum and weights
  • feat: add multisig subcommand to client CLI
  • feat: introduce multisig transaction time-to-live
  • feat: predefine multisig world-level trigger in genesis
  • feat: allow accounts in domain to register multisig accounts

Context

Opens:

Solution

Each commit is explained below, starting with the most recent. You can see the commit history here

feat: support multisig recursion

Allows multisig to function hierarchically, expected to be useful for e.g. automating organizational approval flows.

  • When a multisig transaction proposed, every subordinate node (multisig account and corresponding transactions registry) automatically get ready to accept approvals that will be coming up from individual signatories
  • Using CLI, individual signatories can query all pending multisig transactions they are involved in, not only those proposed to their direct multisig account

Tests:

cargo test -p iroha integration::multisig::multisig_recursion
bash scripts/tests/multisig.recursion.sh

feat: introduce multisig quorum and weights

Inspired by Sui's multisig. Allows for flexible, if not completely free, authentication policies beyond "m of n". For example, weight equivalent to quorum represents administrative privileges

feat: add multisig subcommand to client CLI

$ cargo build
$ ./target/debug/iroha multisig

The subcommand related to multisig accounts and transactions

Usage: iroha multisig <COMMAND>

Commands:
  register  Register a multisig account
  propose   Propose a multisig transaction
  approve   Approve a multisig transaction
  list      List pending multisig transactions relevant to you
  help      Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

You can see more usage in the testing script

feat: introduce multisig transaction time-to-live

Considers the latest block timestamp as the current time and determines timeout, when the transactions registry is called

feat: predefine multisig world-level trigger in genesis

Defines a global trigger in genesis that exercises authority over all domains. There will be three types of triggers on the system side related to multisig:

  • Domains initializer has world-level authority and in response to a domain creation event, registers an accounts registry handled by the domain owner authority
  • Accounts registry has domain-level authority and when called by a domain resident, registers a multisig account along with a transactions registry
  • Transactions registry has account-level authority and when called by a multisig signatory, accepts proposals or approvals for multisig transactions, and is responsible for executing them

feat: allow accounts in domain to register multisig accounts

Accounts registry has authority of the domain owner, so access was previously restricted. This commit allows anyone to organize any multisig account within the domain.

This may be too lenient. Discussion


Review notes

To get an overview,

cargo test -p iroha integration::multisig::multisig
bash scripts/tests/multisig.sh
sequenceDiagram
    autonumber
    participant oo as etc.
    participant DI as Domains Initializer
    Note over DI: /world
    oo-->>DI: domain created
    create participant AR as Accounts Registry
    DI-->>AR: register
    Note over AR: /world/domain
    create actor s0 as signatory 0
    oo->>s0: register
    create actor s1 as signatory 1
    oo->>s1: register
    s0->>AR: request new ms account
    create actor 01 as ms account 01
    AR-->>01: register
    create participant TR as Transactions Registry
    AR-->>TR: register
    AR-->>s0: grant ms role
    AR-->>s1: grant ms role
    Note over 01,TR: /world/domain/account
    s1->>TR: propose instructions
    create participant tx as pending ms transaction
    TR-->>tx: deploy ms transaction
    s0->>TR: approve instructions
    destroy tx
    TR-->>tx: execute instructions
Loading

The dotted line indicates an automatic process

Checklist

  • I've read CONTRIBUTING.md.
  • I've written integration tests for the code changes.
  • All review comments have been resolved.

@github-actions github-actions bot added api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha labels Sep 3, 2024
Copy link

github-actions bot commented Sep 3, 2024

@BAStos525

@s8sato s8sato added the Enhancement New feature or request label Sep 3, 2024
@s8sato s8sato marked this pull request as ready for review September 3, 2024 00:38
@nxsaken
Copy link
Contributor

nxsaken commented Sep 3, 2024

Genesis needs to be updated

scripts/tests/multisig.recursion.sh Outdated Show resolved Hide resolved
client/tests/integration/multisig.rs Outdated Show resolved Hide resolved
tools/kagami/src/genesis/generate.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_accounts/src/lib.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_accounts/src/lib.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_transactions/src/lib.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_domains/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

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

I think this change is great improvement to usability of multisigs.

client/tests/integration/multisig.rs Outdated Show resolved Hide resolved
client/tests/integration/multisig.rs Outdated Show resolved Hide resolved
client/tests/integration/multisig.rs Outdated Show resolved Hide resolved
client_cli/src/main.rs Outdated Show resolved Hide resolved
client_cli/src/main.rs Outdated Show resolved Hide resolved
wasm_samples/executor_custom_data_model/src/multisig.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_accounts/build.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_domains/src/lib.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_domains/src/lib.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_transactions/src/lib.rs Outdated Show resolved Hide resolved
@mversic mversic self-assigned this Sep 4, 2024
tools/kagami/src/genesis/generate.rs Outdated Show resolved Hide resolved
wasm_samples/multisig_domains/src/lib.rs Outdated Show resolved Hide resolved
@s8sato
Copy link
Contributor Author

s8sato commented Oct 3, 2024

Updates:

  • review doc: add a reference to multisig recursion diagram
  • review fix: signatories and weights must be equal in length
  • review fix: condition to rebuild multisig subordinate triggers
  • review fix: genesis.json includes not wasm blobs but hashes
  • review fix: autofill multisig account domains
  • review fix: avoid name collisions of multisig role and triggers
  • review fix: perpetuate multisig domain-level authority
  • review fix: add a domain for domain-free system accounts

commit history

Notes:

  • Currently script tests (scripts/tests/multisig.*) are failing after rebase for some reason

hooks/pre-commit.sample Outdated Show resolved Hide resolved
defaults/genesis.json Outdated Show resolved Hide resolved
crates/iroha/Cargo.toml Outdated Show resolved Hide resolved
@s8sato s8sato force-pushed the feat/4930 branch 2 times, most recently from 44105ac to 35c6e2a Compare October 23, 2024 09:16
Copy link
Contributor Author

@s8sato s8sato left a comment

Choose a reason for hiding this comment

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

Updates:

  • re-export as iroha::multisig_data_model
  • add multisig data model into schema
  • default_executor in wasm/libs
  • early panic with unexpected events
  • rustdoc-compliant warning
  • empty arg to build all wasms
  • fix genesis_block_builder_example test

All changes since the last update contained.

commit history

Notes:

  • genesis_block_builder_example is failing to compile for some reason. Left to another PR

    Thanks to @mversic , fixed

wasm/samples/default_executor/src/lib.rs Outdated Show resolved Hide resolved
scripts/tests/multisig.sh Show resolved Hide resolved
@s8sato s8sato force-pushed the feat/4930 branch 2 times, most recently from e7ea539 to 4e534e8 Compare October 23, 2024 09:57
mversic
mversic previously approved these changes Oct 24, 2024
0x009922
0x009922 previously approved these changes Oct 25, 2024
@s8sato
Copy link
Contributor Author

s8sato commented Oct 25, 2024

Updates:

  • fix CI to build, upload and download wasm libraries
  • FIXUP: make a wasm trigger executable path relative to genesis.json directory
  • FIXUP: hide system entity configurations from kagami::genesis::generate
  • EDIT: review fix: give genesis.json a special field to register wasm triggers

commit history

Notes:

image

  • Does this have something to do with the repository change?

    When an outside contributor submits a pull request to a public repository, a maintainer with write access may need to approve some workflow runs.

    EDIT: resolved by joining the organization

@s8sato s8sato force-pushed the feat/4930 branch 3 times, most recently from 8812567 to 20cba30 Compare October 26, 2024 09:07
@s8sato
Copy link
Contributor Author

s8sato commented Oct 28, 2024

image

This request was automatically failed because there were no enabled runners online to process the request for more than 1 days.

Every failing job runs-on: [self-hosted, Linux, iroha2]. It seems they depend on runners not available atm

BREAKING CHANGES:

- (api-changes) `CanRegisterAnyTrigger` `CanUnregisterAnyTrigger` permissions for system authority
- (api-changes) `GenesisWasmTrigger` in `RawGenesisTransaction` for `genesis.json` readability
- (api-changes) `Multisig*Args` for multi-signature operations
- (config-changes) `genesis.json` assumes `wasm_triggers[*].action.executable` is prebuilt under `wasm_dir`
- (config-changes) docker-compose service generates `genesis.json` by oneself instead of reading bind-mounted one

Major commits:

- feat: support multisig recursion
- feat: introduce multisig quorum and weights
- feat: add multisig subcommand to client CLI
- feat: introduce multisig transaction time-to-live
- feat: predefine multisig world-level trigger in genesis
- feat: allow accounts in domain to register multisig accounts

Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com>
@s8sato s8sato enabled auto-merge (squash) October 29, 2024 05:55
@s8sato s8sato merged commit 40986b3 into hyperledger-iroha:main Oct 29, 2024
15 checks passed
@s8sato
Copy link
Contributor Author

s8sato commented Oct 31, 2024

Afterword: I think it was not a good practice to keep a single commit during review, which was remnants of our Rebase and merge days. Now that we Squash and merge, I'd separate commits to some extent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ergonomic MST
5 participants