Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

EIP-0012 - dApp-Wallet Web Bridge#23

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
rooooooooob wants to merge6 commits intoergoplatform:master
base:master
Choose a base branch
Loading
fromrooooooooob:eip-0012

Conversation

rooooooooob
Copy link

Technical spec for wallet-dApp communication.

Technical spec for wallet-dApp communication.
@rooooooooobrooooooooob marked this pull request as draftAugust 10, 2020 12:47
eip-0012.md Outdated

### Value

BigNum-like object. Represents a value which may or may not be ERG. Value is in the smallest possible unit (e.g. nanoErg for ERG). It can be either a `number` or a `string` using standard unsigned integer text representation.
Copy link
Member

Choose a reason for hiding this comment

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

Value is a positive but encoded as i64 number actually.

Copy link
Member

Choose a reason for hiding this comment

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

@kushti Actually it was a "raw" u64 initially, but now I made a newtypeBoxValue(u64)

Copy link
Author

Choose a reason for hiding this comment

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

If it's positive somewhere (even if Long/i64 encoded), then it should be fine as-is here, right? I didn't look into how Scala handlesLong deserialization from JSON for numbers too big to fit, but in sigma-rust the wrapped u64 should take in a string-encoded uint like "1053546432154" afaik. Please correct me if I'm wrong and this should be changed.

Copy link
Member

Choose a reason for hiding this comment

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

Scala representation of Long is signed, so max value is 2^63-1.

Copy link
Author

Choose a reason for hiding this comment

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

For JSON deserialization in the full node is this description fine though? Wouldn't the max possible value for nanoergs be much smaller than this? I guess for other token types it could be relevant - in which case should we keep it as-is but specify that it cant' be more than a 64-bit signed number or what?

@gagarin55
Copy link
Contributor

I suggest you to specify data types in language independent way, not types from sigma-rust implementation

@rooooooooob
Copy link
Author

I suggest you to specify data types in language independent way, not types from sigma-rust implementation

We do specify the types in language-independant JSON. This was intended to mirror what sigma-rust had at the time for ease of interopability, but they are specified at the bottom of the document.

…ementation of it in Yoroi.Type definitions are specified using flow types now instead of somepseudo JS schema.API errors are added to all API calls to handle things that can go wrongfor any call,e.g. internal error in the wallet, incorrect paramsspecified to API calls.get_change_address() was added as an explicit API since it should beeasy to support and allows for an easier way during tx creation ratherthan arbitrarily choosing an address from the used or unused API calls,which aren't garuanteed to be non-empty.add_external_box was removed as we are still unsure how any walletswould be able to implement it in any meaningful way. This could bereversed and have most wallets treat it as a no-op.
@rooooooooob
Copy link
Author

I made some minor changes based on issues encountered during our initial implementation inside of Yoroi:

Type definitions are specified using flow types now instead of some
pseudo JS schema.

API errors are added to all API calls to handle things that can go wrong
for any call,e.g. internal error in the wallet, incorrect params
specified to API calls.

get_change_address() was added as an explicit API since it should be
easy to support and allows for an easier way during tx creation rather
than arbitrarily choosing an address from the used or unused API calls,
which aren't guaranteed to be non-empty.

add_external_box() was removed as we are still unsure how any wallets
would be able to implement it in any meaningful way. This could be
reversed and have most wallets treat it as a no-op.

We removedBoxCandidate in favor of justBox to make things simpler. It also allows interop with sigma-rust/full nodes for now.

It would be ideal to not reference sigma-rust in any way for things that are more specific than just the underlying javascript type for example:

Uses `sigma-rust` rep - Hex-encoded bytes for `SigmaSerialize` of a constant.

which specify things like encoding, length, etc.

The id field has been removed, as well as the tx id/box id from outputswhich are now BoxCandidates.This was only used before to help with interop with exisitng tooling butnow that sigma-rust no longer requires these there's no reason to keepthem around. They only made it more difficult to construct an unsignedtransaction. Now one can be a lot more easily constructed even withoutusing other tooling to calculate the IDs/etc.
@rooooooooob
Copy link
Author

BoxCandidate was restored as it was only removed to ease support with sigma-rust/for simplicity, but the additional information was never necessary. Now that the data model was updated in sigma-rust it makes more sense to revert that change to the originalBoxCandidate model and remove tx IDs from the unsignedTx which makes it potentially easier to construct by the dApp.

Full box contents are now included inUnsignedTransaction to help light wallets support P2S inputs + for all wallets in processing recently created outputs. Wallets can check that the content matches the ID so there should be no security issues with that.

typo in type nameCo-authored-by: Denys Zadorozhnyi <denys@zadorozhnyi.com>

### Box

Transaction output box. Format is the same as `sigma-rust`:
Copy link
Member

Choose a reason for hiding this comment

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

There is noBox in sigma-rust. I supposeErgoBox is meant here.


A candidate (no TX/box IDs) for an output in an `UnsignedTransaction`
```
type BoxCandidate = {|
Copy link
Member

@greenhatgreenhatJul 22, 2021
edited
Loading

Choose a reason for hiding this comment

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

Probably should mention that it'sErgoBoxCandidate insigma-rust?


### SignedTx

Uses `sigma-rust` rep for a transaction:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Uses`sigma-rust` rep for a transaction:
Uses`sigma-rust``Transaction`rep for a transaction:

@rooooooooob
Copy link
Author

We're currently also developing a similar spec to this for Cardanohere. There has been a lot of discussion there, some of which is relevant to Ergo. Here are some things that we might want to cover here too:

  • Handling multiple wallets installed (for Cardano we solve this by injecting into a namespaced object - for Ethereum they just tell you to only have one installed e.g. MetaMask)
  • Versioning (i.e. if this spec gets updated and some API changes)
  • Phishing prevention (this could be assumed to be a wallet implementation detail, e.g. showing some user-known secret image in connector popups to stop phishing sites from creating similar looking popups to a user's wallet to farm passwords or other info)


The proposed API is limited to just the minimum wallet <-> dApp communication needed rather than providing lots of utility functions (tx building, data conversions, etc) or node-access (for UTXO scanning). This is different compared to web3 as that functionality could be modular in a separate library and Ergo smart contracts don't need as much direct node access for basic dApp functionality compared to Ethereum.

This API is accessible from a javascript object `ergo` that is injected into the web context upon wallet consent. Before this just the free `ergo_request_read_access()` and `ergo_check_read_access()` functions are injected into the web context. An event handler can also be registered to the web context to detect wallet disconnects tagged as `"ergo_wallet_disconnected"`.

Choose a reason for hiding this comment

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

In order to avoid conflicts with multiple wallets, can we do a slight change on the access API? Maybe something likeergo.{walletName}.requestReadAccess() andergo.{walletName}.checkReadAccess()

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that's what we ended up doing for CIP-0030 for Cardano which I based off of this. I'm not sure if it's fine to do such a breaking change at this point since this has been up for a long time although it's still a draft PR. I'll wait and see what the Ergo devs think about this. We also don't have any functionality for versioning either. I mentioned both of these in my comment above in August. There are additional events that can be of use for dApps as well that could be added.

capt-nemo429 reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

Agree with@capt-nemo429 . Explicitly distinguishing wallets in the dApp connector API is a crucial feature. Shouldn't be too problematic to migrate.

capt-nemo429 and TanBeige reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@greenhatgreenhatgreenhat left review comments

@kushtikushtikushti requested changes

@SebastienGllmtSebastienGllmtSebastienGllmt left review comments

@oskin1oskin1oskin1 left review comments

@capt-nemo429capt-nemo429capt-nemo429 left review comments

@MrStahlfelgeMrStahlfelgeMrStahlfelge requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@rooooooooob@gagarin55@greenhat@kushti@SebastienGllmt@MrStahlfelge@oskin1@capt-nemo429

[8]ページ先頭

©2009-2025 Movatter.jp