-
Notifications
You must be signed in to change notification settings - Fork 249
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
doc: examples for Near-related host functions #1259
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1259 +/- ##
=======================================
Coverage 80.16% 80.16%
=======================================
Files 102 102
Lines 14598 14598
=======================================
Hits 11703 11703
Misses 2895 2895 ☔ View full report in Codecov by Sentry. |
@denbite please fix compilation errors. There's at least one, where |
@dj8yfo done, please review |
…th reference to implementation repo example
@dj8yfo also just added a little text about the purpose of functions like |
@race-of-sloths score 13 |
@denbite Thank you for your contribution! Your pull request is now a part of the Race of Sloths! Current status: executed
Your contribution is much appreciated with a final score of 13! @dj8yfo received 25 Sloth Points for reviewing and scoring this pull request. Congratulations @denbite! Your PR was highly scored and you completed another monthly streak! To keep your monthly streak make another pull request next month and get 8+ score for it What is the Race of SlothsRace of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! Bot commands
|
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.
@denbite it felt somewhat overwhelming to request changes for all found suggestions
with current pr not introducing anything particularly erroneous,
so i'll extract those to an issue in case someone volunteers to further improve docs by adding links
/// # Examples | ||
/// ``` | ||
/// use near_sdk::env::signer_account_pk; | ||
/// use near_sdk::{PublicKey, CurveType}; | ||
/// | ||
/// let pk = PublicKey::from_parts(near_sdk::CurveType::ED25519, vec![0; 32]).unwrap(); |
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.
suggest using ```no_run annotation
and using a public key, which looks real,
and not the one tied to MockedBlockchain::default()
result,
( equivalent to calling testing_env!(VMContextBuilder::new().build());
)
/// ``` | ||
/// use near_sdk::env::input; | ||
/// | ||
/// assert_eq!(input(), Some(Vec::new())); | ||
/// ``` |
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.
/// ``` | |
/// use near_sdk::env::input; | |
/// | |
/// assert_eq!(input(), Some(Vec::new())); | |
/// ``` | |
/// ```no_run | |
/// use near_sdk::env::input; | |
/// | |
/// // contract had some input | |
/// assert_eq!(input(), Some(vec![0u8, 31, 54, 38, 22])); | |
/// ``` |
/// ``` | ||
/// use near_sdk::env::block_index; | ||
/// | ||
/// assert_eq!(block_index(), 0); |
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.
suggest using ```no_run and some realistic value from current testnet
/mainnet
/// ``` | ||
/// use near_sdk::env::block_height; | ||
/// | ||
/// assert_eq!(block_height(), 0); |
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.
suggest using ```no_run and some realistic value from current testnet
/mainnet
/// ``` | ||
/// use near_sdk::env::block_timestamp; | ||
/// | ||
/// assert_eq!(block_timestamp(), 0); |
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.
suggest using ```no_run and some realistic value from current testnet
/mainnet
@@ -948,6 +1721,19 @@ pub fn storage_read(key: &[u8]) -> Option<Vec<u8>> { | |||
} | |||
/// Removes the value stored under the given key. | |||
/// If key-value existed returns `true`, otherwise `false`. | |||
/// |
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.
should mention usage in
https://github.com/near/near-sdk-rs/tree/master/near-contract-standards/src/upgrade/mod.rs
/// | ||
/// # Use cases | ||
/// Storage functions are typically used to upgrade/migrate a contract state, preventing errors like `Cannot deserialize the contract state` after rolling out the breaking changes to the network. | ||
/// For practical examples, see different implementations in [this repository](https://github.com/near-examples/update-migrate-rust). |
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.
this call isn't used in the referenced repo, and also in a pending pr near-examples/update-migrate-rust#2
won't request change to remove this link, hoping that this and some of other suggestions will be adressed in some of subsequent prs
@@ -956,10 +1742,34 @@ pub fn storage_remove(key: &[u8]) -> bool { | |||
} | |||
} | |||
/// Reads the most recent value that was evicted with `storage_write` or `storage_remove` command. | |||
/// | |||
/// # Use cases | |||
/// Storage functions are typically used to upgrade/migrate a contract state, preventing errors like `Cannot deserialize the contract state` after rolling out the breaking changes to the network. |
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.
this call isn't used in the referenced repo, and also in a pending pr near-examples/update-migrate-rust#2
won't request change to remove this link, hoping that this and some of other suggestions will be adressed in some of subsequent prs
/// | ||
/// # Use cases | ||
/// Storage functions are typically used to upgrade/migrate a contract state, preventing errors like `Cannot deserialize the contract state` after rolling out the breaking changes to the network. | ||
/// For practical examples, see different implementations in [this repository](https://github.com/near-examples/update-migrate-rust). |
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.
this call isn't used in the referenced repo, and also in a pending pr near-examples/update-migrate-rust#2
won't request change to remove this link, hoping that this and some of other suggestions will be adressed in some of subsequent prs
@@ -996,7 +1806,14 @@ pub fn state_exists() -> bool { | |||
|
|||
/// Price per 1 byte of storage from mainnet genesis config. | |||
/// TODO: will be using the host function when it will be available. | |||
|
|||
/// |
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.
should mention usage in
https://github.com/near/near-sdk-rs/tree/master/near-contract-standards/src/fungible_token/storage_impl.rs
https://github.com/near/near-sdk-rs/tree/master/near-contract-standards/src/non_fungible_token/utils.rs
https://github.com/near/near-sdk-rs/tree/master/examples/fungible-token/tests/workspaces.rs
Make it straightforward for developers how host functions can be imported and used
The final markdown doc page will appear as follows