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

docs: document callback_unwrap attribute #1313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TobieTom
Copy link

@TobieTom TobieTom commented Feb 19, 2025

Fixes #1303

Adds documentation for the callback_unwrap attribute under the #[near] macro docs, including:

  • Description of functionality and usage
  • Integration with PromiseOrValue
  • Cross-contract factorial example
  • Error handling alternatives
  • Rust doc annotations and references

Resolves #1303


For more details, open the Copilot Workspace session.

Fixes near#1303

Adds documentation for the `callback_unwrap` attribute under the `#[near]` macro docs, including:

- Description of functionality and usage
- Integration with `PromiseOrValue`
- Cross-contract factorial example 
- Error handling alternatives
- Rust doc annotations and references

Resolves near#1303

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/near/near-sdk-rs/issues/1303?shareId=XXXX-XXXX-XXXX-XXXX).
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.26%. Comparing base (9596835) to head (542e467).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1313      +/-   ##
==========================================
+ Coverage   80.22%   80.26%   +0.03%     
==========================================
  Files         104      104              
  Lines       14844    14844              
==========================================
+ Hits        11909    11914       +5     
+ Misses       2935     2930       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dj8yfo dj8yfo left a comment

Choose a reason for hiding this comment

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

overall this is the right direction, but all examples should at least compile (or run).

the second one isn't very clear what it's about, and it doesn't compile, which is a sign
that it's incorrect and/or incomplete.

this can be iterated quicker locally with cargo test --doc near-sdk/src/lib.rs

Comment on lines +317 to +336
/// ### Error Handling Alternative
///
/// If you need to handle failed promises explicitly, receive the `Result<T, PromiseError>` directly
/// instead of using `#[callback_unwrap]`:
///
/// ```rust
/// # use near_sdk::{near, PromiseError};
/// # #[near(contract_state)]
/// # pub struct Contract {}
/// # #[near]
/// # impl Contract {
/// #[private]
/// pub fn handle_callback(&mut self, result: Result<u8, PromiseError>) {
/// match result {
/// Ok(value) => { /* Handle success */ },
/// Err(err) => { /* Handle error */ },
/// }
/// }
/// # }
/// ```
Copy link
Collaborator

Choose a reason for hiding this comment

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

this example looks a superficial copy-paste from elsewhere, and it's not very clear how it's an alternative one to the first heading ### Example with Cross-Contract Factorial, or what else it is an alternative to.

if it's indeed an alternative to the first heading, it should accomplish the same purpose as first heading, but without using the #[callback_unwrap], thus illustrating the difference

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TobieTom i suggest that you remove this example completely, as from what i can see the Result<u8, PromiseError> is only supposed to be used together with another attribute #[callback_result]

        #[callback_result] c: Result<u8, PromiseError>,

Comment on lines +286 to +287
/// This attribute is commonly used with [`PromiseOrValue<T>`], which represents either an immediate value
/// or a promise that will resolve to that value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// This attribute is commonly used with [`PromiseOrValue<T>`], which represents either an immediate value
/// or a promise that will resolve to that value.
/// This attribute is commonly used with [Promise] or [`PromiseOrValue<T>`] as the return type of another contract method,
/// whose return value will be passed as argument to `#[callback_unwrap]`-annotated argument

Copy link
Collaborator

Choose a reason for hiding this comment

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

this reference+applied_suggestion can stay, despite the #1313 (comment) comment

Comment on lines +311 to +313
/// pub fn factorial_mult(&self, n: u32, #[callback_unwrap] cur: u32) -> u32 {
/// n * cur
/// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

for task #1312 (NOT in scope of current pr) it can be mentioned that the attribute works by doing the following:

#[no_mangle]
pub extern "C" fn factorial_mult() {
 ....
    let data: ::std::vec::Vec<u8> = match ::near_sdk::env::promise_result(0u64) {
        ::near_sdk::PromiseResult::Successful(x) => x,
        _ => ::near_sdk::env::panic_str("Callback computation 0 was not successful"),
    };
    let cur: u32 = match ::near_sdk::serde_json::from_slice(&data) {
        Ok(deserialized) => deserialized,
        Err(_) => ::near_sdk::env::panic_str("Failed to deserialize callback using JSON"),
    };
    let contract: CrossContract = ::near_sdk::env::state_read().unwrap_or_default();
    let result = CrossContract::factorial_mult(&contract, n, cur);
    
    ...

with links to corresponding [near_sdk::env] host-functions used

/// .into()
/// }
///
/// #[private]
Copy link
Collaborator

Choose a reason for hiding this comment

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

it can be mentioned with link to #[private] attribute's anchor that this is marked as private method to allow only calling
factorial_mult from factorial contract method by CrossContract itself and disallow it to be called externally by users

/// return PromiseOrValue::Value(1);
/// }
/// let account_id = env::current_account_id();
/// Self::ext(account_id.clone())
Copy link
Collaborator

@dj8yfo dj8yfo Feb 19, 2025

Choose a reason for hiding this comment

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

EDIT: discard this comment completely

This should mention that Self::ext creates an instance of <ContractType>Ext structure
(this reference would/should be resolved in scope of #1308, but is not available yet as a link).

As the documentation for <ContractType>Ext isn't ready yet and in order for this pr to not introduce dependencies by explaining concepts with other completely unexplained concepts,

this should mention that Self::ext creates CrossContractExt struct, which has the versions of factorial and factorial_mult returning [Promise] -es, which should be enough for the purposes of #[callback_unwrap] description

impl CrossContractExt {
    pub fn factorial(self, n: u32) -> ::near_sdk::Promise {
        ...
        
    }
    pub fn factorial_mult(self, n: u32) -> ::near_sdk::Promise {
        ...
        
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TobieTom in order to avoid explaning higher-level concepts with other unexplained higher-level primitives (Self::ext, #1308) , i suggest you replace the first and only example with

#[near]
impl CrossContract {
    pub fn factorial(&self, n: u32) {
        if n <= 1 {
            env::value_return(&serde_json::to_vec(&1u32).unwrap());
            return;
        }
        let account_id = env::current_account_id();
        let prepaid_gas = env::prepaid_gas().saturating_sub(FACTORIAL_CALL_GAS);
        let promise0 = env::promise_create(
            account_id.clone(),
            "factorial",
            &serde_json::to_vec(&(n - 1,)).unwrap(),
            NearToken::from_near(0),
            prepaid_gas.saturating_sub(FACTORIAL_MULT_CALL_GAS),
        );
        let promise1 = env::promise_then(
            promise0,
            account_id,
            "factorial_mult",
            &serde_json::to_vec(&(n,)).unwrap(),
            NearToken::from_near(0),
            FACTORIAL_MULT_CALL_GAS,
        );
        env::promise_return(promise1);
    }

    /// Used for callbacks only. Multiplies current factorial result by the next value. Panics if
    /// it is not called by the contract itself.
    #[private]
    pub fn factorial_mult(&self, n: u32, #[callback_unwrap] factorial_n_minus_one_result: u32) -> u32 {
        log!("Received n: {:?}", n);
        log!("Received factorial_n_minus_one_result: {:?}", factorial_n_minus_one_result);
        let result = n * factorial_n_minus_one_result;
        log!("Multiplied {:?}", result.clone());
        result
    }
}

and mention with references that [env::promise_create], [env::promise_then] and [env::promise_return] are used in factorial method to set up a callback of factorial_mult with result of factorial .

this definetely works (adjusted to some imports required) . i can see

            logs: [
                "Received n: 5",
                "Received factorial_n_minus_one_result: 24",
                "Multiplied 120",
            ],

in the log

/// }
/// }
/// ```
///
Copy link
Collaborator

Choose a reason for hiding this comment

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

should also mention 4 in-repo examples with doc-links <> such as <https://github.com/near/near-sdk-rs/blob/master/examples/callback-results/src/lib.rs#L51>

# ran in ./examples sub-folder of repo root: grep callback_unwrap -R .
./cross-contract-calls/high-level/src/lib.rs:    pub fn factorial_mult(&self, n: u32, #[callback_unwrap] factorial_n_minus_one_result: u32) -> u32 {
./adder/src/lib.rs:        #[callback_unwrap] a: DoublePair,
./adder/src/lib.rs:        #[callback_unwrap] b: DoublePair,
./callback-results/src/lib.rs:        #[callback_unwrap] a: u8,

@@ -277,6 +277,63 @@ extern crate quickcheck;
/// }
/// ```
///
/// ## `#[callback_unwrap]` (annotates function parameters)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The whole anchor/heading of this attribute should be blaced after #[handle_result] attribute's anchor

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

Successfully merging this pull request may close these issues.

[Task] document callback_unwrap
2 participants