Skip to content

rfc: interface between wallet and sdk - #1614

Closed
lklimek wants to merge 1 commit into
feat/registerfrom
rs-sdk/wallet-interface
Closed

lklimek wants to merge 1 commit into
feat/registerfrom
rs-sdk/wallet-interface

Conversation

@lklimek

@lklimek lklimek commented Dec 4, 2023 •

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

For the rs-sdk testing, we assume that each feature must have relevant test implemented.
Current rs-sdk test framework doesn't support testing of Puts (what includes state transitions).

For example, for testing of Put Identity, rs-sdk MUST be able to execute the following operations:

  1. Create new identity - OK
  2. Create state and sign state transition - not possible
  3. Send state transition to DAPI - OK
  4. Retrieve state transition result from the DAPI - OK
  5. Verify state transition on the server side - OK

In order to create state and sign state transition, the Sdk needs code that is currently part of wallet in rs-platform-explorer, expecially fn registration_transaction(). Suggested change is as follows:

  • main logic of registration_transaction() shall be moved to rs-sdk, and included inside the put_* methods
  • rs-platform-explorer shall implement the Wallet trait that implements access to wallet as a set of atomic (eg. doing only one thing) functions
  • wallet shall NOT provide access to private key - it should provide respective sign method instead, with enough details to ensure the Wallet can verify tx and is not blindly signing data - I assume separate sign function might be needed for different types of transitions. These sign methods are not part of this PR.

Rationale

In rs-sdk we have the following architecture:

  • simple services layer - layer providing "atomic" features, eg. doing only one thing at a time; for example, FromProof trait or rs-dapi-client
  • composite services layer - layer connecting these simple services together, to do more complex stuff; this is the role of rs-sdk

Now, I see the wallet as a member of simple services layer, managing addresses and keys - that is, more-or-less CRUD-type operations ("read that address balance", "create new address", etc.). IMO it should have a sub-component that will be a "secure cryptographic device" (ledger/smartcard/trusted chip in phone/...)., but this is out of scope of what we do now.

In this architecture, the role of rs-sdk shall be to compose complex services from simple services - in this particular case, a composite service is implemented in registration_transaction , and IMO it should be part of composite services layer == rs-sdk.

The reason "why" comes from my belief that in most cases, the typical app developer doesn't care about selecting utxo's, determining change addresses, outputs, etc. In essence, the developer doesn't care about all these wallet internals. His goal is to create an identity, and he wants to do it in 2-3 lines, instead of having to understand state transitions, asset locks, utxos, fees, signatures, etc. To me, it all looks confusing when I try to do that, after years of exp in Dash. For newcomers, it's a show-stopper.

However, if a developer wants more control, he should reach out to "simple services" layer and do implementation himself. That's why we designed sdk this way.

So, the goal is to make developer's life as easy as one-two screens of code to create an identity.

What was done?

Draft wallet and example of use in an application - not building yet.

How Has This Been Tested?

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek

lklimek commented Dec 12, 2023

Copy link
Copy Markdown
Contributor Author

Closed in favor of #1569

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.

1 participant