iroha icon indicating copy to clipboard operation
iroha copied to clipboard

feat!: recognize and activate accounts

Open s8sato opened this issue 1 year ago • 8 comments

BREAKING_CHANGE:

  • Account contains is_active field
  • AccountEvent::Created divides into Recognized and Activated
  • ValidationFail contains UnrecognizedAuthority and InactiveAuthority variants

Description

When an account is recognized,

  • it becomes able to hold assets, permissions, roles, and metadata
  • it cannot yet execute any queries or transactions

When an account is activated,

  • it becomes able to execute queries and transactions

Recognize and activate an account when targeted as:

  • Register<Account> object

Recognize an account when targeted as:

  • Register<Asset> object.account
  • Mint<Numeric, Asset> destination.account
  • ~~Transfer<Account, DomainId, Account> destination~~ See comment
  • Transfer<Account, AssetDefinitionId, Account> destination
  • Transfer<Asset, Numeric, Account> destination
  • Transfer<Asset, Metadata, Account> destination
  • SetKeyValue<Account> object
  • SetKeyValue<Asset> object.account
  • Grant<Permission, Account> destination
  • Grant<Role, Account> destination

Let me call these creative instructions in documentation

Linked issue

  • closes #4426

Benefits

  • who targets an account does not need to care if it has been registered or not
  • depending on use case, accounts can be registered implicitly/automatically or explicitly/manually

Checklist

  • [x] I've read CONTRIBUTING.md
  • [x] I've used the standard signed-off commit format (or will squash just before merging)
  • [ ] All applicable CI checks pass (or I promised to make them pass later)
  • [ ] (optional) I've written unit tests for the code changes
  • [ ] I replied to all comments after code review, marking all implemented changes with thumbs up

s8sato avatar May 30 '24 03:05 s8sato

This is not how I would implement it. I would rather just control the access through executor as follows:

  1. By default all instructions that target accounts are allowed:
  • this covers public use
  • requires no executor (public case shouldn't require executor)
  1. Custom executor rejects instructions that target accounts that have not yet been registered:
  • this covers private use
  • should be part of our default executor

I also believe this approach reduces the implementation complexity.

I don't think there is any need to support inactive accounts in private blockchains. In private blockchains accounts first have to get registered then they can be used. The is_active limbo doesn't bring anything for private case

mversic avatar Jun 06 '24 10:06 mversic

As I understand it, according to @mversic your suggestion, the process when an account is targeted is as follows:

  • public use
    • the target is immediately registered no matter what domain the target belongs to
      • may be reasonable, since the domain boundaries may not make sense in public chains
  • private use
    • as before, only registered accounts can be valid targets

This means that no intermediate state is needed for account registration. If so, let's remove is_active flag and forget about recognize/activate terms.

Now, there are two possible approaches in terms of granularity of the account auto-registration policy:

  1. core processes back to executor for auto-registering the target
    • executor is responsible for registering targeted accounts
      • returns uniform results regardless of authority of the instruction
    • chain level public/private characterization
    • needs a flag to indicate if Register<Account> is called back from core for auto-registration
  2. each domain has a flag, and core determines whether to auto-register the target from its domain, without processing back to executor
    • core is responsible for registering targeted accounts
    • domain level public/private characterization
    • how should the flag be modified, if it should be?

Dealing with the concept of domains and public/private uses at the same time is new to me, and I seek further input

s8sato avatar Jun 07 '24 07:06 s8sato

  • the target is immediately registered no matter what domain the target belongs to

wasn't that how you implemented, or was there something different?

as before, only registered accounts can be valid targets

yes

This means that no intermediate state is needed for account registration. If so, let's remove is_active flag and forget about recognize/activate terms.

yes

mversic avatar Jun 07 '24 07:06 mversic

  1. core processes back to executor for auto-registering the target

can't executor make sure account is registered before calling isi.execute(), i.e. handing over execution to the core

mversic avatar Jun 07 '24 07:06 mversic

I don't think there is any need to support inactive accounts in private blockchains.

What about offline transactions? do you think they are required only in public case?

Is full registration even possible automatically?

I mean that executor/trigger might have not enough data to perform registration (i.e. metadata)

Erigara avatar Jun 11 '24 06:06 Erigara

Requirement for auto registration emerge from our implementation details (account structs holds assets).

Shouldn't we reconsider this?

In this case we would allow isi where target account is not registered yet and if required we can prohibit this isi by checking if account is registered in the executor.

Erigara avatar Jun 11 '24 06:06 Erigara

The feature might require the separate and relational entities after all. As I understand it, an example where a new account "carol" comes to exercise its authority in public use is as follows:

  1. the world has separate assets and accounts maps

    - world:
      - numeric_assets:
        - rose:
          - alice: 13
      - accounts:
        - alice:
          - metadata:
    
  2. (public use) alice transfers 3 roses to carol, who is not registered yet

    - world:
      - numeric_assets:
        - rose:
          - alice: 10
          - carol:  3 # appeared
      - accounts: # not checked when carol is targeted
        - alice:
          - metadata:
    
  3. (public use) carol registers herself

    - world:
      - numeric_assets:
        - rose:
          - alice: 10
          - carol:  3
      - accounts:
        - alice:
          - metadata:
        - carol: # registered
          - metadata:
    
  4. carol successfully transfers 2 roses to alice

    - world:
      - numeric_assets:
        - rose:
          - alice: 12
          - carol:  1
      - accounts: # checked when carol is authority
        - alice:
          - metadata:
        - carol:
          - metadata:
    

I'll extend this idea to other ISIs and other entity maps and consider consistency. For example, I can immediately see that SetKeyValue<Account> would be invalid before account registration

s8sato avatar Jun 13 '24 09:06 s8sato

What about offline transactions? do you think they are required only in public case?

IMHO yes, but that should be confirmed. In private use-case it's expected users are registered before becoming entities in the network

mversic avatar Jun 14 '24 09:06 mversic

So the idea is to deliberately introduce an inconsistency, the absence of a primary entity in ACCOUNTS in this diagram while other storages referencing it, and thereby represent a pre-registration state of the account. I'll convert this PR to draft (and maybe close it) and resume after #4792

s8sato avatar Jul 02 '24 05:07 s8sato

Closed as the issue should be addressed by a different approach such as https://github.com/hyperledger/iroha/pull/4668#issuecomment-2165073957

s8sato avatar Aug 05 '24 08:08 s8sato