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

Dapi for N3 #145

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Dapi for N3 #145

wants to merge 27 commits into from

Conversation

erikzhang
Copy link
Member

No description provided.

@erikzhang erikzhang marked this pull request as ready for review November 24, 2021 09:22
@lllwvlvwlll
Copy link
Member

I'm a proponent of standardizing the protocol as long as it does not impact the transport layer of any solution. Ultimately, most of this interfacing will be handled by sdks which abstract a lot of this so its primarily a standard for tooling developers.

I would also like to request that we generalize the name of this standard since dApi is already a software implementation which also includes a transport layer.

nep-20.mediawiki Outdated

In order for dapps to have a unified method of obtaining <code>IDapiProvider</code> instances, all providers must trigger the <code>Neo.DapiProvider.ready</code> event of the <code>window</code> object when they are ready.

The front end of the dapp can listen to the event and obtain the <code>IDapiProvider</code> instance from the <code>detail</code> property of the event.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for web based dapp clients, but what about mobile or desktop clients?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is difficult for mobile clients and desktop clients to have a unified method to obtain IDapiProvider objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't have to be a unified method, but I'd expect there to be some method for mobile and desktop apps to access a DAPI provider

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any ideas?

Copy link

Choose a reason for hiding this comment

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

WalletConnect 2.0 handles this using a "connection code": It could open the wallet via Deeplinking or show the code as an QRCode image, and then, on the Wallet the user can confirm the connection. So, for this to work, we would need a new event, so the dapp can handle this UI interaction to show the code.

nep-20.mediawiki Outdated

==Rationale==

This protocol will allow dApp developers to create applications that interact with the NEO blockchain without having to be concerned about managing a full wallet within their application or the details related to handing transaction creation or broadcasting. This will also allow dApps to allow users to transact in a secure fashion that does not require sharing of their private key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended to be a protocol or an API? the specification above appears to be a Typescript API, but here it's described as a protocol (which would imply the possibility of multiple implementations in different programming languages)

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, the front end of most dapps is Javascript running in browsers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, the front end of most dapps is Javascript running in browsers.

This doesn't answer my "is this intended to be a protocol or api" question

Copy link
Member Author

Choose a reason for hiding this comment

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

The specification describes the interfaces, so it is a protocol. Different wallet providers can have different implementations, but the interaction protocol with dapps is the same.

@melanke
Copy link

melanke commented Dec 1, 2021

Hello, I just have a few minor considerations regarding this specification:

I've tested Dapi with NeoLine in the past and also by checking it's documentation, I understood that the Chrome Extension doesn't need a proper "connection"
process, the dapp receives the wallet information automatically.

  1. This is very different from WalletConnect 2.0: the dapp will need to show a code for the user to input on the wallet, this can be done via QRCode or Deeplinking, so it would be interesting to have such event to allow the dapp to handle such UI interaction.

  2. On this specification proposal there is an authenticate method. I am not very sure if it was designed with the connection purpose, to continue the discussion I will assume it was. So, here is my first question: What is the purpose of the AuthenticationChallengePayload.nonce property?

  3. The Network enum doesn't allow tests on a private network. And AuthenticationChallengePayload.network only accepts a single network, which is fine on most of the cases, but we tought that it would be interesting for the dapp to accept connecting with multiple networks, so the user on it's wallet can control if it will be used for tests, private network or production. This would be great for development purposes, it would make dapp development way easier, testing on the private network before sending to testnet or mainnet.

  4. Again, on the connection process, WalletConnect expects the Dapp's meta-data. This is used to show the dapp's name, icon and other information on the wallet. The metadata has this format:

export type DappMetadata = {
    name: string
    description: string
    url: string
    icons: string[] // honestly, we could allow a single icon, so it don't need to be an array
}

I believe it could be an interesting addition.

  1. Both NeonWallet's WalletConnect integration and NeoLine's Dapi integration have the following methods that I consider important and could be added to the specification:
  • invokeMultiple. On NW we are accepting an optional parameter on each invocation called abortOnFail: it adds 0x39 on the script, so the whole transaction will be aborted if this invocation returns false, which might be very important for transactional operations.
  • signMessage
  1. The following methods are less important because they could be called directly from the dapp, without the need of the wallet integration, but they are pretty handy, specially to avoid a Neo javascript SDK dependency on the dapp. On NW we are calling those methods without the need for the user authorization, since they are pretty secure, so the dapp and the wallet has a smooth interaction:
  1. Being able to easily convert an Address or ScriptHash to hash160 would also be pretty handy. Both NeoLine and NeonWallet handle this differently but we could agree on a single method.

@erikzhang
Copy link
Member Author

erikzhang commented Dec 4, 2021

On this specification proposal there is an authenticate method.

Many websites require users to log in to provide services. The authenticate method provides an identity authentication scheme for users to log in to websites using blockchain addresses.

verifyMessage

I think it should be provided in SDK.

invokeRead

It has been renamed to call.

Being able to easily convert an Address or ScriptHash to hash160 would also be pretty handy.

I think it should be provided in SDK.

@devhawk
Copy link
Contributor

devhawk commented Dec 4, 2021

What is the relationship of this to WalletConnect? Are they complementary or competitive?

@erikzhang
Copy link
Member Author

What is the relationship of this to WalletConnect? Are they complementary or competitive?

Dapi is a protocol for communication between plug-in wallet and dapp via javascript. And WalletConnect allows communication between the two through a centralized server to relay messages.

nep-20.mediawiki Outdated
// Represents the name of the event in IDapiProvider.
export type EventName =
"accountschanged" | // Triggered when the accounts in the wallet change.
"networkchanged"; // Triggered when the user switches the current network.
Copy link

Choose a reason for hiding this comment

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

Could we have a new event for connectionProposal that could be emitted to deliver a connection code? The dapp could show it as QRCode to stablish a connection. This is very important for mobile Wallets.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't quite understand the role of connectionProposal. Can you draw a flowchart to explain it?

Copy link

Choose a reason for hiding this comment

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

  • Hey, I just made this, I hope it could be helpful:

wallet-connect-sequence-diagram


  • And also, I wrote this while I was investingating it:

Wallet Connect implementation

Wallet Connect is a protocol to allow a safer communication between dApp and Wallet. It works with any blockchain because it simply creates a WebSocket server that handles the communication between dApp and Wallet in a smart way, handling format validation, problems with internet connection, cache, log, etc. It's client-side libraries helps to integrate even further, allowing the user to scan a QR Code to connect the two ends.

The parts

Server

A simple middleman that handles the communication between the other parts. At first, we believe we would need to change Wallet Connect relay server so it could work with Neo Blockchain and then send a pull request. But after exploring its code, it looks like we can simply use Wallet Connect server as it is right now.

dApp

A dApp is an application that uses smart contracts and other technologies as server-side. To securely reach the Smart Contracts, the user must use its own Wallet to sign the operations, and the responsible to delegate such interaction is the dApp's client-side. Firstly, the dApp establishes a connection with Wallet Connect which asks the user to scan the QR Code, by doing this, Wallet Connect knows how to make an end-to-end encryption and transfer information between the dApp and the Wallet. Now, the dApp can request to do an operation on the blockchain using the user's Wallet. By using JsonRPC format, it says which method and arguments it wants to call.

Wallet

The Wallet is a client-side application that allows the user to handle its assets, sign documents and interact with Smart Contracts. If integrated with Wallet Connect, it can receive messages from dApps that requests the user to procced with a blockchain operation. When receiving such methods, the Wallet should ask for user authorization and then call the blockchain method by itself. A good UX pattern is to show a mobile notification so the user can allow it without having to open the app by himself.

Copy link
Member Author

Choose a reason for hiding this comment

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

As I said:

Dapi is a protocol for communication between plug-in wallet and dapp via javascript. And WalletConnect allows communication between the two through a centralized server to relay messages.

They are different.

Copy link

Choose a reason for hiding this comment

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

Sure, but they focus solving the same problem. Are we going to exclude the possibility for WalletConnect integration on this specification? I mean, if the dapp wants to have multiple dapp-wallet integration providers, WalletConnect couldn't be one of them? Meaning that there will be absolutely no mobile wallet providers.

nep-20.mediawiki Outdated Show resolved Hide resolved
@EdgeDLT
Copy link

EdgeDLT commented Dec 11, 2021

I think it should be possible for a dapp to provide a signature to a transaction created using the Dapi. From what I understand it should not increase risk for users, only for dApps that need to be careful they don't provide a signature to a malicious transaction.

My imagined use case is for subsidizing transactions as the first signer, but I imagine there are others situations where apps might want to join in a multisig or approve certain contract calls.

@erikzhang
Copy link
Member Author

I think it should be possible for a dapp to provide a signature to a transaction created using the Dapi.

maketransaction and sign are designed for this situation.

signMessage(message: Base64Encoded, account?: UInt160): Promise<{ signature: Base64Encoded; account: UInt160; pubkey: ECPoint }>;

}
</pre>
Copy link
Member

Choose a reason for hiding this comment

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

How about ask wallet to add Network and Switch to another Network?

Copy link
Member

Choose a reason for hiding this comment

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

I would like that the authors of the proposals checked that and solved the issues pending. Maybe it is needed or not.

@Jim8y Jim8y requested a review from a team May 24, 2024 02:39
Copy link
Contributor

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

I've never used it, so just some nitpicking. Likely we need more input from providers/consumers of this API.

export type Address = string;

// A 160-bit hash represented by a hexadecimal string.
// Example: "0x682cca3ebdc66210e5847d7f8115846586079d4a"
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it doesn't have a 0x prefix, is it a valid UInt160?

export type UInt160 = string;

// A 256-bit hash represented by a hexadecimal string.
// Example: "0x1f4d1defa46faa5e7b9b8d3f79a06bec777d7c26c4aa5f6f5899a291daa87c15"
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it doesn't have a 0x prefix, is it a valid UInt256?

// Example: "03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c"
export type ECPoint = string;

// A large integer of any length expressed as a string.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is 1e8 a valid Integer?

export type HexString = string;

// Represents a version.
// Example: "1.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific requirements? SemVer?

"accountschanged" | // Triggered when the accounts in the wallet change.
"networkchanged"; // Triggered when the user switches the current network.

// Represents the type of ContractParameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

NEP-14 should be referenced here.

type: ContractParameterType; // The type of the parameter.
}

// Represents an account in a wallet.
Copy link
Contributor

Choose a reason for hiding this comment

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

NEP-6 should be referenced here.

txid: UInt256;
executions: {
trigger: TriggerType;
vmstate: VMState;
Copy link
Contributor

Choose a reason for hiding this comment

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

abortOnFail?: boolean; // Indicates whether the entire transaction should fail when the contract returns `false`.
}

export type InvocationResult = {
Copy link
Contributor

Choose a reason for hiding this comment

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

notifications?

// Possible errors: UNSUPPORTED, INVALID, NOTFOUND, TIMEOUT, CANCELED.
sign(context: ContractParametersContext): Promise<ContractParametersContext>;

// Signs the message with the specified account.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some specifics there usually like prefix added, it should be mentioned.


This protocol will allow dApp developers to create applications that interact with the NEO blockchain without having to be concerned about managing a full wallet within their application or the details related to handing transaction creation or broadcasting. This will also allow dApps to allow users to transact in a secure fashion that does not require sharing of their private key.

==Implementation==
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

From my last review I believe that this #145 (comment) was pending

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants