feat: added `map` method to query builders #45

akorchyn · 2025-01-30T12:25:36Z

added map method to change the query result type if you want to construct it some other way around. These methods help to simplify mapping internally (more clean) and provide a big improvement in dev UX on fetching data.
Bug-fixed Delegation::view_balance method that returned staked balance instead of unstaked.

* Documented types. * Bug-fixed viewing balance of the account by calculating manually storage cost

race-of-sloths · 2025-01-30T12:26:22Z

@akorchyn Thank you for your contribution! Your pull request is now a part of the Race of Sloths!
We are launching a survey "Who is faster? You vs SpaceX"

Current status: waiting for merge

Reviewer	Score
@dj8yfo	8

Your contribution is much appreciated with a final score of 8!
You have received 80 Sloth points for this contribution

@dj8yfo received 25 Sloth Points for reviewing and scoring this pull request.

What is the Race of Sloths

Race 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:

Tag @race-of-sloths inside your pull requests
Wait for the maintainer to review and score your pull request
Check out your position in the Leaderboard
Keep weekly and monthly streaks to reach higher positions
Boast your contributions with a dynamic picture of your Profile

For maintainers:

Score pull requests that participate in the Race of Sloths and receive a reward
Engage contributors with fair scoring and fast responses so they keep their streaks
Promote the Race to the point where the Race starts promoting you
Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands

For contributors
- Include a PR: @race-of-sloths include to enter the Race with your PR
For maintainers:
- Invite contributor @race-of-sloths invite to invite the contributor to participate in a race or include it, if it's already a runner.
- Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
- Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
- Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

dj8yfo

this thingy here has to test the same thing that will be published
RUSTDOCFLAGS="-D warnings" cargo doc --all-features --document-private-items

https://github.com/near/near-api-rs/blob/main/.github/workflows/test.yml#L43

akorchyn · 2025-02-11T20:24:42Z

this thingy here has to test the same thing that will be published RUSTDOCFLAGS="-D warnings" cargo doc --all-features --document-private-items

https://github.com/near/near-api-rs/blob/main/.github/workflows/test.yml#L43

Don't think it related to this PR, though

dj8yfo · 2025-02-12T12:11:09Z

Don't think it related to this PR, though

actually , i went to workflows in the target branch to copy an up-to-date command to a local justfile to build documentation with, to look at doc comments added in target branch, and noticed it's out of sync with docs.rs directives in manifest,
so i think it's pretty much related to this pr, despite the referenced line being nowhere close to modified ones

besides, the commented structs/methods (e.g. in src/common/query.rs) reside in a private module, so their addition won't be checked for consistency in ci with cargo doc before attempting to publish

dj8yfo

@race-of-sloths score 8
this looks great

left some comments

dj8yfo · 2025-02-11T19:21:25Z

src/common/query.rs

+/// - [Tokens::ft_balance](crate::tokens::Tokens::ft_balance)
+/// - [StakingPool::staking_pool_info](crate::stake::Staking::staking_pool_info)


these should hint that implementations are meant, or better be replaced with compilable example (without map logic, to showcase stock MultiRpcBuilder)

this MultiRpcBuilder as well as its alias near_api::common::query::MultiQueryBuilder can be considered to be made public (they don't appear to be public atm), as i see no reason why lib user cannot try to combine something like https://docs.rs/near-api/latest/src/near_api/tokens.rs.html#179-217 on his/her own, if he wants to try something custom

dj8yfo · 2025-02-11T19:57:49Z

src/common/query.rs

@@ -173,6 +181,22 @@ where
        }
    }

+    /// Map response of the queries to another type. The `map` function is executed after the queries are fetched.
+    ///
+    /// The response is a tuple of the responses of the queries.


this line can be made more specific, by replacing response -> with Handler:Response ,
replacing the queries -> the querires added by [Self::add_query] ,

and also the statement about a tuple of responses can specify, that for a tuple of the responses
a MultiQueryHandler<(H1, ... Hn)> has to be used to initialize the Handler of this MultiRpcBuilder.

let postprocess = MultiQueryHandler::new(( CallResultHandler(PhantomData::<Response1>), CallResultHandler(PhantomData::<Response2>), ));

It's also worth mentioning that currently MultiQueryHandler supports tupples with sizes in the range 1..=3 (not absolutely sure about that, but that was my impression)

a compilable example to showcase this api in isolation from other api (ft_balance) in the lib would be beneficial too

dj8yfo · 2025-02-12T16:30:30Z

src/tokens.rs

+            CallResultHandler(PhantomData::<FungibleTokenMetadata>),
+            CallResultHandler(PhantomData::<U128>),


the only field of tuple struct could be made non-pub, as formally using a PhantomData<T> is an implementation detail

impl<Response: Send + Sync> CallResultHandler<Response> { pub fn new() -> Self { Self(PhantomData::<Response>) } }

let postprocess = MultiQueryHandler::new(( CallResultHandler::<FungibleTokenMetadata>::new(), CallResultHandler::<U128>::new(), // CallResultHandler(PhantomData::<FungibleTokenMetadata>), // CallResultHandler(PhantomData::<U128>), ));

maybe this could be streamlined further to only require

let postprocess = MultiQueryHandler::new(Default::default());

but this remains unclear, whether it's a good idea, specifying types upfront looks readable,
though maybe a tad redundant

dj8yfo · 2025-02-12T16:54:47Z

src/storage.rs

+                block_height: storage.block_height,
+                block_hash: storage.block_hash,


this could be a map or map_inner_data method on https://docs.rs/near-api/latest/near_api/types/struct.Data.html itself

dj8yfo · 2025-02-12T17:30:48Z

src/stake.rs

+                }),
+            )?
+            .read_only()
+            .map(near_data_to_near_token))
    }


these 3 could be further dedup-ed, as they only differ in called function method_name

dj8yfo · 2025-02-12T17:37:45Z

src/stake.rs

                let staked = near_data_to_near_token(staked);
                let unstaked = near_data_to_near_token(unstaked);
                let total = near_data_to_near_token(total);


frankly speaking, this looks strange. not sure why this compiles.
One would expect that 3 queries returned by add_query_builder already resolve to NearToken,
as they all have .map(near_data_to_near_token)) in their implementation bodies and have NearToken return type somewhere in their method signatures.

first impression that it should be just composing fields into output type

UserStakeBalance { staked, unstaked, total, }

without the need for an extra round of near_data_to_near_token type conversions

dj8yfo · 2025-02-12T18:01:09Z

src/common/query.rs

@@ -147,6 +147,14 @@ pub type MultiQueryBuilder<T> = MultiRpcBuilder<T, RpcQueryRequest, BlockReferen
 pub type ValidatorQueryBuilder<T> = RpcBuilder<T, RpcValidatorRequest, EpochReference>;
 pub type BlockQueryBuilder<T> = RpcBuilder<T, RpcBlockRequest, BlockReference>;



i don't know, this module could be split into some smaller sub-modules, maybe defined by some core trait that all items within a sub-module implement, maybe by some other categories (maybe by a few layers of categories) ,
but the names are all a bit lookalike , QueryBuilder, RpcBuilder, RpcHandler, RpcRequest, RpcRequestHandler, ...

idk, if e.g. RpcBuilder resided in near_api::common::query::<module_name>, and QueryBuilder resided in
near_api::common::query::<module_name>::aliases, maybe these would be easier to separate from MultiRpcBuilder and its alias

it's not large though, only 800 lines

dj8yfo · 2025-02-12T18:46:06Z

src/stake.rs

+            .add_query_builder(Self::staking_pool_reward_fee(pool.clone()))
+            .add_query_builder(Self::staking_pool_delegators(pool.clone()))
+            .add_query_builder(Self::staking_pool_total_stake(pool))
+            .map(move |(reward_fee, delegators, total_stake)| {
                let total = near_data_to_near_token(total_stake);


same thing here abount apparently repeated near_data_to_near_token

akorchyn added 10 commits January 27, 2025 16:41

doc!: documented types

95d086a

* Documented types. * Bug-fixed viewing balance of the account by calculating manually storage cost

cspell support

592477d

cargo doc fix

a0d1548

restored locked field

98f0e64

review

8c887ed

typo

d305fe5

feat: read-only-with-postprocess

0ad4623

simplified the contract interaction internally

25750cd

documentation

6b84430

Merge branch 'main' into simplify-contract-reuse

1fc337c

akorchyn requested review from dj8yfo and PolyProgrammist as code owners January 30, 2025 12:25

replaced method read_only_with_mapping to map method on query builder

42a02f8

akorchyn changed the title ~~feat: added read_only_with_postprocess method to simplify contract interaction~~ feat: added map method to query builders Feb 3, 2025

akorchyn and others added 2 commits February 3, 2025 22:08

ci

a258db7

Merge branch 'main' into simplify-contract-reuse

9138e85

dj8yfo reviewed Feb 11, 2025

View reviewed changes

dj8yfo approved these changes Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added `map` method to query builders #45

feat: added `map` method to query builders #45

akorchyn commented Jan 30, 2025 •

edited

Loading

race-of-sloths commented Jan 30, 2025 •

edited

Loading

dj8yfo left a comment

akorchyn commented Feb 11, 2025

dj8yfo commented Feb 12, 2025 •

edited

Loading

dj8yfo left a comment

dj8yfo Feb 11, 2025

dj8yfo Feb 11, 2025

dj8yfo Feb 11, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

dj8yfo Feb 12, 2025

		/// - [Tokens::ft_balance](crate::tokens::Tokens::ft_balance)
		/// - [StakingPool::staking_pool_info](crate::stake::Staking::staking_pool_info)

		CallResultHandler(PhantomData::<FungibleTokenMetadata>),
		CallResultHandler(PhantomData::<U128>),

		block_height: storage.block_height,
		block_hash: storage.block_hash,

		@@ -147,6 +147,14 @@ pub type MultiQueryBuilder<T> = MultiRpcBuilder<T, RpcQueryRequest, BlockReferen
		pub type ValidatorQueryBuilder<T> = RpcBuilder<T, RpcValidatorRequest, EpochReference>;
		pub type BlockQueryBuilder<T> = RpcBuilder<T, RpcBlockRequest, BlockReference>;

feat: added map method to query builders #45

Are you sure you want to change the base?

feat: added map method to query builders #45

Conversation

akorchyn commented Jan 30, 2025 • edited Loading

race-of-sloths commented Jan 30, 2025 • edited Loading

dj8yfo left a comment

Choose a reason for hiding this comment

akorchyn commented Feb 11, 2025

dj8yfo commented Feb 12, 2025 • edited Loading

dj8yfo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: added `map` method to query builders #45

feat: added `map` method to query builders #45

akorchyn commented Jan 30, 2025 •

edited

Loading

race-of-sloths commented Jan 30, 2025 •

edited

Loading

dj8yfo commented Feb 12, 2025 •

edited

Loading