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

Add multi-issuer support for wallets #46

Merged
merged 6 commits into from
May 28, 2024

Conversation

cmickeyb
Copy link
Contributor

@cmickeyb cmickeyb commented May 9, 2024

This PR implements a multi-issuer wallet. That is, a wallet that can manage (and perform operations on) assets from multiple issuers. The wallet implements several widgets that can be used to import new issuers, check on the balance of holdings at multiple issuers (each with a unique identity), and transfer assets to other holdings (including identities with only a public key available, meaning other users running in other isolated containers).

The PR documents (or updates) three different test scenarios for the wallet:

  • 06.single_user_wallet -- simple single issuer test
  • 07.multiple_issuer_wallet -- test with a single wallet including multiple issuers
  • 08.multier_user_wallet -- test with multiple wallets each with multiple issuers

Note that this PR does not include explicit contract flush at this point in time. That means that when refreshing the balance, it may require up to 30 seconds for the contract state to flush and be refreshed.

This should be tested with PDO PR #491 being applied: hyperledger-labs/private-data-objects#491

@cmickeyb cmickeyb requested a review from prakashngit May 9, 2024 22:52
@cmickeyb cmickeyb force-pushed the mic.apr29.multiuser branch 2 times, most recently from ddc0cd1 to 648fe45 Compare May 16, 2024 19:06
@cmickeyb cmickeyb removed their assignment May 16, 2024
const int count = (int) msg.get_number("count");
ASSERT_SUCCESS(rsp, count > 0, "invalid transfer request, invalid asset count");

// if there is no issuance for this identity, we treat it as a 0 balance
ww::exchange::LedgerEntry old_entry;
ASSERT_SUCCESS(rsp, ledger_store.get_entry(old_owner, old_entry),
"transfer failed, insufficient balance for transfer");
ASSERT_SUCCESS(rsp, count < old_entry.asset_.count_,
ASSERT_SUCCESS(rsp, count <= old_entry.asset_.count_,
"transfer failed, insufficient balance for transfer");
Copy link
Contributor

@prakashngit prakashngit May 22, 2024

Choose a reason for hiding this comment

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

Given that we permit transferring all "counts" of the old owner to the new owner (and thus making asset_.count = 0 for the old owner), wondering why in line 89 (as part of the issue method), we do not permit issuing "zero" counts. I would imagine this is perhaps equivalent to creating a bank account for someone with "zero balance"? I guess an independent question is "is there a notion of bank for the transfer to happen?" or can the transfer method be done on any identities?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm not sure i understand the question... semantically, we assume that there is an entry for every identity though for most the balance is 0 (there is no notion of "create an account"). however, MOST 0 balance accounts are not explicitly tracked.

this specific fix allows for all funds from an account to be transferred. we don't explicitly delete the account (this is one of the cases where an account may have a 0 balance).

Refactor the contract and exchange jupyter modules into separate
directory structures. This will enable more future expansion especially
for application specific jupyter code for the exchange contracts.

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
This completes the functionality for test
07.multiple_issuer_wallet.md. That is, a user can create a wallet that
provides access to multiple issuers with functions to import new
issuers, check the balance of accounts from multiple issuers, and
transfer assets that reside in multiple issuers.

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
* Fix a balance comparison bug
* Fix a bug with self transfers

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
Update the test scripts for multiuser wallets. The tests reflect the
steps necessary to use the widgets interface in the wallet notebook.
In addition, a new test 08.mutiple_user_wallet.md describes a test
that creates two wallets and transfers assets between them.

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
Refresh with no services selected would generate an
exception in the group selection/creation widget.
This just fixes the initialization.

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
Expose and use the necessary interfaces for tokens to be included in
wallet notebooks. The big benefit of this is that all "ownership"
functions can now be handled through the same interface whether they
are fungible assets (like marbles) or non-fungible assets (like
tokens).

Needed to support the "get_balance" method in the token contract (we
already support many of the other issuer methods through
tokens). Added better support for the balance and transfer in the
plugins.

And... most notably, added an "issuer" reference in the context
file. Context references turn out to be rather useful. This creates a
reference to the specific token object in the collections file. So now
there is a canonical reference point for accessing issuer functions.

Signed-off-by: Mic Bowman <mic.bowman@intel.com>
@cmickeyb
Copy link
Contributor Author

With the updates to the PDO branch, this should now be complete. That is, the necessary changes to PDO are available in the submodule.

@prakashngit prakashngit merged commit 7668a57 into hyperledger-labs:main May 28, 2024
2 checks passed
@cmickeyb cmickeyb deleted the mic.apr29.multiuser branch November 6, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants