-
Notifications
You must be signed in to change notification settings - Fork 142
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
Contract Self-Identification NEP #129
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the part about auditing can potentially be its own NEP as there are a lot of potential problems there. Without a proper reputation system it seems that having the auditor's signature isn't really useful.
Why does it have to be on-chain? When you initialize a contract you pass a link to the metadata into the initialization argument and let's say the contract shouldn't change this link. The metadata can contain all the required info. Audits can also go off-chain and link to a particular metadata file. |
I'm open to all suggestions. I'm putting all on-chain so you don't depend on nothing else than the chain, I'm with the idea that the more we use the chain for, the better (Maybe due to lack-of-knowledge I'm being too enthusiastic about it). From Devs POV is better because you don't have to deal with other protocols/storage/concepts/documentation. If we use Github or IPFS, Devs need to read about/understand those services and also your code needs to include API's/libs to access those services. If everything is on-chain, you can get the data with the same near-cli/api you're already using. |
@bowenwang1996 Yes, the user must previously select auditors she trusts. But it's chicken-and-egg, this NEP could make the existence of a reputation system for auditors a necessity. |
My main reason for why not on-chain is that the most information doesn't have to be on-chain. For example the contracts don't care about audits information, so it doesn't have to be on-chain so long as we can link this information to the contract. It can be linked if each audit has the account-id and the contract code hash for what's reviewed. If we can securely link metadata file and the deployed contract, then we can put all required info into the metadata file. If one contract wants to verify on-chain that the other contract satisfies the standard, by relying on an audit. This can be done if there is an auditor contract that whitelists contracts instead. Then you can rely on this whitelist to give you a verification that the given contract satisfies requirements. It's similarly how the Staking pool contracts are whitelisted for lockup contracts to delegate. Eventually, all metadata can live on-chain and the link can just point to a contract on chain. But for now the audits have overhead and there are no need to validate them on-chain. |
Heavy simplification: Based on reasonable feedback I've simplified the NEP limiting it to just contract self-identification and reducing on-chain required space to a minimum. |
@luciotato should we also add URL with canonical web app for given contract? This can be useful for wallet a lot. /cc @kcole16 |
Good idea @vgrichina. Done. |
I started a discussion with an alternative approach: interface registry: #154 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments with regard to the format of the document (syntax only)
@@ -0,0 +1,207 @@ | |||
# Contract Self-Identification ([NEP-129](https://github.com/nearprotocol/NEPs/pull/129)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Rationale and Alternative version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Copyright?
Prior art: | ||
- TBD | ||
|
||
## Guide-level explanation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Guide-level explanation | |
## Specification |
## Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
### Reference Implementations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Reference-level explanation | |
[reference-level-explanation]: #reference-level-explanation | |
### Reference Implementations: | |
## Reference Implementations |
} | ||
``` | ||
|
||
### Details on the fields: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Details on the fields: | |
### Details on the fields |
Remove two points for headers
} | ||
``` | ||
|
||
#### ts/pseudocode: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion, consider moving, ts implementation to assets, and link it here (mostly if it is pseudocode).
#### Rust: | ||
```rust | ||
// --------------------------------- | ||
// -- CONTRACT Self Identification -- | ||
// --------------------------------- | ||
// [NEP-129](https://github.com/nearprotocol/NEPs/pull/129) | ||
// see also pub fn get_contract_info | ||
pub const CONTRACT_NAME: &str = "diversifying staking pool"; | ||
pub const CONTRACT_VERSION: &str = "0.1.0"; | ||
pub const DEVELOPERS_ACCOUNT_ID: &str = "developers.near"; | ||
pub const DEFAULT_WEB_APP_URL: &str = "http://div-pool.narwallets.com"; | ||
pub const DEFAULT_AUDITOR_ACCOUNT_ID: &str = "auditors.near"; | ||
|
||
/// NEP-129 get information about this contract | ||
/// returns JSON string according to [NEP-129](https://github.com/nearprotocol/NEPs/pull/129) | ||
/// Rewards fee fraction structure for the staking pool contract. | ||
#[derive(Serialize)] | ||
#[serde(crate = "near_sdk::serde")] | ||
#[allow(non_snake_case)] | ||
pub struct NEP129Response { | ||
pub dataVersion:u16, | ||
pub name:String, | ||
pub version:String, | ||
pub developersAccountId:String, | ||
pub source:String, | ||
pub standards:Vec<String>, | ||
pub webAppUrl:Option<String>, | ||
pub auditorAccountId:Option<String>, | ||
} | ||
|
||
/// NEP-129 get information about this contract | ||
/// returns JSON string according to [NEP-129](https://github.com/nearprotocol/NEPs/pull/129) | ||
pub fn get_contract_info(&self) -> NEP129Response { | ||
return NEP129Response { | ||
dataVersion:1, | ||
name: CONTRACT_NAME.into(), | ||
version:CONTRACT_VERSION.into(), | ||
developersAccountId:DEVELOPERS_ACCOUNT_ID.into(), | ||
source:"https://github.com/Narwallets/diversifying-staking-pool".into(), | ||
standards:vec!("NEP-129".into()), | ||
webAppUrl:self.web_app_url.clone(), | ||
auditorAccountId:self.auditor_account_id.clone() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the current code distracting. I think either pseudo code is better (potentially moving this implementation to assets) and making sure it compiles (if copy pasted properly in the project).
Otherwise, let's make sure the code is properly formatted, get rid of global variables given they are used only once, properly update comments so they are more useful.
version:CONTRACT_VERSION.into(), | ||
developersAccountId:DEVELOPERS_ACCOUNT_ID.into(), | ||
source:"https://github.com/Narwallets/diversifying-staking-pool".into(), | ||
standards:vec!("NEP-129".into()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
standards:vec!("NEP-129".into()), | |
standards:vec!["NEP-129".into()], |
## Changelog | ||
|
||
### `0.1.0` | ||
|
||
- Heavy simplification. Only contract identification. | ||
- added webAppUrl per @vgrichina suggestion | ||
|
||
### `0.0.1` | ||
|
||
- Initial Proposal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove. Add vgrichina as coauthor if necessary.
There are two alternative to this NEP: #154 and #275. Both of them are very similar, they propose to create an on-chain registry with the information of which NEP is supported by each contract. Advantage of registry vs builtin self-identification:
Con:
There is almost no difference about having the information in a registry vs inside the contract itself for discoverability. Off-chain tools can be built either way. For on-chain contracts that want to discover if a particular standard is implemented, they will need to make a xcc, and react on that information on the callback. It is not particularly relevant if the queried contract is the target contract or a registry. |
Contract Self-Identification (NEP-129)
Initial Draft
see: https://github.com/luciotato/NEPs/blob/contract-self-identification/specs/Standards/Contracts/SelfIdentification.md