api spec by sekulicd · Pull Request #1 · vulpemventures/ocean

sekulicd · 2021-12-28T12:20:22Z

This adds api spec of the wallet daemon.
Purpose of a daemon is to run as a sidecar for other applications and removes 'heavy burden' of maintaining internal wallet.
Protobuf files are splitted into 4 files that I see as main features of the wallet.
Wallet -> covers creation/restore of wallet
Account -> covers creation on addresses, tracking balance and uspents for specific account
Transaction -> used for crafting various types of transactions
Notification -> defines rpc's that are to be used for fetching notification.

@tiero @altafan please review.

tiero · 2021-12-28T12:56:30Z

Makefile

+.PHONY: proto doc
+
+## proto: compile proto files
+proto:


shall we start with buf.build?

tiero · 2021-12-28T12:58:31Z

api-spec/protobuf/account.proto

+  int64 account_index = 1;
+}
+
+message UpdateRequest{


This is only to update the name? not changes to derivations?

better to specify in the comment then (since we generate doc from it)

tiero · 2021-12-28T13:17:06Z

api-spec/protobuf/account.proto

+  // Asset in bytes or asset_commitment if value is blinded
+  bytes asset = 3;
+  // Value in bytes or value_commitment if value is blinded
+  bytes value = 4;


I prefer to have a dedicate field for each, when blinded we leave this empty, when unblinded we should return both.

tiero · 2021-12-28T13:18:22Z

api-spec/protobuf/account.proto

+  uint64 target_amount = 3;
+  // Unspents selection algorithm.
+  enum Strategy {
+    RANDOM = 0;


Suggested change

RANDOM = 0;

BRANCH_BOUND = 0;

FRAGMENT =1;

the onw

tiero · 2021-12-28T15:36:42Z

api-spec/protobuf/notification.proto

+  bytes asset = 3;
+  // Value in bytes or value_commitment if value is blinded
+  bytes value = 4;


tiero · 2021-12-28T15:38:18Z

Just to keep in mind, Conceptually we can load the wallet also in a watch-only mode ie. no signing keys on board, only xpub(s) and master blinding key.

Maybe an RPC in Wallet service that gives us this info could be helpful

tiero · 2021-12-28T15:39:08Z

api-spec/protobuf/wallet.proto

+    Open is used to open existing HD Wallet based on password so that daemon can serve
+    other calls
+   */
+  rpc Open (OpenRequest) returns (OpenResponse);


Suggested change

rpc Open (OpenRequest) returns (OpenResponse);

rpc Unlock (OpenRequest) returns (OpenResponse);

tiero · 2021-12-28T17:02:44Z

api-spec/protobuf/wallet.proto

+message CreateRequest{
+  // signing_mnemonic is 24 length phrase used to derive addresses and scripts
+  string signing_mnemonic = 1;
+
+  // blinding_mnemonic is 24 length phrase used to derive blinding keys
+  string blinding_mnemonic = 2;
+
+  // password is used to encrypt HD Wallet wallet. After creation, password is required to unlock the wallet daemon.
+  bytes  password = 3;
+}


we should specify the derivation path here

what about descriptor?

I do think should instead be under Account, and be a dedicate RPC: ideally CreateAccount derives new xpub, with that result we pass it to compose our descriptor/miniscript/ionio in a second UpdateScriptTemplate kind of.

tiero · 2021-12-30T16:10:00Z

api-spec/protobuf/account.proto

+}
+message CreateAccountReply{
+  // Account index of the derivation path.
+  int64 account_index = 1;


Add the resulting xpub for the account here

Also the derivation path of the xpub for completeness?

tiero · 2021-12-30T16:12:06Z

api-spec/protobuf/account.proto

+  AccountKey account_key = 1;
+  // Name of the account to be updated.
+  string name = 2;
+}


Add here a Template template field, to accept descriptor, miniscript, ionio, raw opcode templates etc..

tiero · 2021-12-30T16:15:50Z

api-spec/protobuf/account.proto

+  enum Type {
+    P2WPKH = 0;
+    P2TR = 1;
+  }
+  Type type = 3;


replace Type with template field

tiero · 2021-12-30T16:18:27Z

api-spec/protobuf/types.proto

+  // list of unspents
+  repeated Unspent unspents = 2;
+}
+


message Template { enum Format { DESCRIPTOR = 0, MINISCRIPT = 1, IONIO = 2, RAW =3 } Format format = 1; string value; }

altafan

CreatePsbt, Mint, ReMint, Burn, Transfer and ClaimPegin, they all suppose the service to use or select utxos to be used as inputs of a transaction.

Shall all these utxos be marked as locked until the tx is not detected as broadcasted or unconfirmed to prevent the wallet from using them for other requests in the meantime?

altafan · 2021-12-30T17:46:46Z

api-spec/protobuf/wallet.proto

+    Seed is used to generate signing and blinding seed that are to be passed
+    to Create rpc which will create HD Wallet
+  */
+  rpc Seed(SeedRequest) returns (SeedReply);


GenSeed more clear?

altafan · 2021-12-30T17:47:47Z

api-spec/protobuf/wallet.proto

+  string signing_mnemonic = 1;
+
+  // blinding_mnemonic is 24 length phrase used to derive blinding keys
+  string blinding_mnemonic = 2;


shouldn't these be fields of the reply?

altafan · 2021-12-30T17:48:44Z

api-spec/protobuf/wallet.proto

+  rpc Seed(SeedRequest) returns (SeedReply);
+
+  // CreateWallet is used to create HD Wallet based on signing, blinding seeds and password
+  rpc CreateWallet(CreateWalletRequest) returns (CreateWalletResponse);


convenience is to use Reply suffix for returned messages

altafan · 2021-12-30T17:49:36Z

api-spec/protobuf/wallet.proto

+  rpc ChangePassword (ChangePasswordRequest) returns (ChangePasswordResponse);
+
+  // Restore is used to restore HD Wallet based on signing and blinding seeds
+  rpc Restore (RestoreRequest) returns (RestoreResponse);


name should be RestoreWallet to be consistent with CreateWallet

altafan · 2021-12-30T17:50:37Z

api-spec/protobuf/types.proto

+  // Tx id of the unspent
+  string tx_id = 1;


Suggested change

// Tx id of the unspent

string tx_id = 1;

// Txid of the unspent

string txid = 1;

altafan · 2021-12-30T18:23:58Z

api-spec/protobuf/account.proto

+}
+message CreateAccountReply{
+  // Account index of the derivation path.
+  int64 account_index = 1;


Also the derivation path of the xpub for completeness?

altafan · 2021-12-30T18:26:29Z

api-spec/protobuf/account.proto

+  // list of account's
+  repeated AccountInfo accounts = 1;
+}
+message AccountInfo {


missing xpub and derivation path?

altafan · 2021-12-30T18:29:20Z

api-spec/protobuf/types.proto

+  repeated Unspent unspents = 2;
+}
+
+message Unspent {


missing field for status (un)confirmed

maybe missing a field for status (un)locked?

altafan · 2021-12-30T18:29:44Z

api-spec/protobuf/types.proto

+  // Value commitment, empty if value is not confidential
+  bytes value_commitment = 6;
+  // Script
+  bytes script = 7;


would also add a string field for the address to easily get the related singing/blinding info if needed.

altafan · 2021-12-30T18:37:16Z

api-spec/protobuf/account.proto

+  // Unspents grouped by address
+  map<string, Unspents> unspents = 1;


Maybe grouping unspents based on their (un)confirmed status would make more sense?

What about those unspents used for example in a not yet broadcasted Mint tx (ins are automatically selected by the service)?
Should they be recognized as "locked"?
If yes, should they be returned here?
If yes, then we should group all unspents based on their (un)confirmed or locked status.

api spec

d6e549d

tiero requested a review from altafan December 28, 2021 12:54

tiero reviewed Dec 28, 2021

View reviewed changes

Makefile Outdated

.PHONY: proto doc

## proto: compile proto files

proto:

Copy link

Member

tiero Dec 28, 2021

Choose a reason for hiding this comment

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

shall we start with buf.build?

tiero reviewed Dec 28, 2021

View reviewed changes

add buf.build

e6cf763

tiero reviewed Dec 30, 2021

View reviewed changes

altafan requested changes Dec 30, 2021

View reviewed changes

sekulicd mentioned this pull request Dec 31, 2021

Api spec v2 #2

Merged

tiero closed this Dec 31, 2021

-    RANDOM = 0;
+    BRANCH_BOUND = 0;
+    FRAGMENT =1;

	rpc Open (OpenRequest) returns (OpenResponse);
	rpc Unlock (OpenRequest) returns (OpenResponse);

		// Unspents grouped by address
		map<string, Unspents> unspents = 1;

Conversation

sekulicd commented Dec 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiero commented Dec 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiero Dec 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiero Dec 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

altafan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

altafan Dec 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiero Dec 29, 2021 •

edited

Loading

tiero Dec 30, 2021 •

edited

Loading

altafan Dec 30, 2021 •

edited

Loading