feat: add new provider api and tests

[hal3e]

closes: #605 and #350

I have updated provider with the new APIs and added respective unit tests.

[iqdecay]

Looking really good

[Salka1988]

👍

[digorithm]

👍

[Br1ght0ne]

👍

[hal3e]

I have added test for the missing APIs: transactions,transactions_by_owner, block and blocks. For the block and blocks test I am not sure if we can test them in a better way. I was not able to find another way to get the block_id.

[hal3e]

Maybe this PR would be a good place to discuss if the user should be able to change the pagination parameters. Currently, most of the APIs will return all available elements e.g. coins, transactions etc. This could be a problem on, for example, the testnet where we have a lot of coins or transactions.
One solution would be to have the user enter an Option<u64> that should represent the max returned number of items from the client. For example, get_transactions(Some(100)). This would return up to 100 transactions. None could mean return all.

[hal3e]

In the last commit, I have changed the key of the hashmap that the balances APIs return. Previously, this was an asset_id hex_string and now it is an AssetId. IMO this makes the UX much better as we do not have to do conversions. The conversion is done once in the API. Let me know if you like this. This is a breaking change (I have updated the docs)
example:

// old way
let balances: HashMap<String, u64> = wallet.get_balances().await?;
let asset_id_hex_key = format!("{:#x}", asset_id);
balances.contains_key(&asset_id_hex_key);

// new way
let balances: HashMap<AssetId, u64> = wallet.get_balances().await?;
assert!(balances.contains_key(&asset_id));

[segfault-magnet]

Maybe this PR would be a good place to discuss if the user should be able to change the pagination parameters. Currently, most of the APIs will return all available elements e.g. coins, transactions etc. This could be a problem on, for example, the testnet where we have a lot of coins or transactions. One solution would be to have the user enter an Option<u64> that should represent the max returned number of items from the client. For example, get_transactions(Some(100)). This would return up to 100 transactions. None could mean return all.

I've been pondering pagination as well.

What I would like to avoid:

• Guessing the max num of records at any time and adapting the page size to it
• Having the page size be too big -- either the user doesn't need soo many or, more importantly, it will put a strain on the node
• Having an API that forces its user to exhaust all of the queried resources. e.g. returning Vec<T> -- besides potentially taking forever, the fact that there might be multiple network calls hidden behind the API is not apparent.

While working on #598 I came up with something like this:

fn paginate_all<T, F, Fut, S>(
    supplier: F,
    request_template: PaginationRequest<S>,
) -> impl TryStream<Ok = T, Error = ProviderError>
where
    F: Fn(PaginationRequest<S>) -> Fut,
    Fut: Future<Output = Result<PaginatedResult<T, S>, ProviderError>>,
    S: Clone,
{
    let supplier = Rc::new(supplier);
    let template = request_template;
    stream::try_unfold((true, None), move |state| {
        let supplier = Rc::clone(&supplier);
        let template = template.clone();
        async move {
            let (has_next_page, cursor) = state;

            if has_next_page {
                let request = PaginationRequest { cursor, ..template };

                supplier(request)
                    .await
                    .map(|e| Some((e.results, (e.has_next_page, e.cursor))))
            } else {
                Ok(None)
            }
        }
    })
    .map_ok(|e| stream::iter(e).map(Ok))
    .try_flatten()
    .into_stream()
}

You would use it like so:

        let request_template = PaginationRequest::<String> {
            cursor: None,
            results: 100,
            direction: PageDirection::Forward,
        };
        let a = paginate_all(
            |req| async move { self.get_transactions_by_owner(address, req).await },
            request_template,
        )
        .map_ok(|resp| resp.transaction.id().to_string())
        .map_ok(move |id: String| async move {
            self.client
                .receipts(&id)
                .await
                .map_err(ProviderError::ClientRequestError)
        })
        .try_buffered(max_concurrent_requests.unwrap_or(5))
        .try_collect::<Vec<_>>()
        .await
        .unwrap();

paginate_all accepts, what it calls, a supplier. That is any lambda that is willing to accept a PaginationRequest and will return a PaginatedResult.

This allows you to wrap paginate_all over any graphql-like paginable API.

But it returns a TryStream which resembles an Iterator but adapted for the async world.

The above result amounts to the following behavior:

• I'm using paginate_all and describing to it that I want to paginate the transactions_by_owner API and to paginate with the given template -- i.e. go FORWARD 100 requests at a time.
• paginate_all will not return chunks of 100 but rather flatten them out so any subsequent map calls will get the items one by one, as if they're iterating through a collection
• Since streams are lazy, just like iterators, nothing will happen unless somebody starts consuming the elements.
• Once the first 100 elements are consumed, paginate_all will proceed to acquire the next 100.
• One by one I extract the id from the incoming transactions and do another query, albeit an unpaginated one, for the receipts of that transaction.
• I'm doing 5 transactions at a time, 5, 10..15.. until we exhaust the 100 and receive a fresh batch of 100 transactions.
• The end consumer will see receipts coming at him one at a time even though getting one is a multi-step process.

The stream API is powerful enough that even if I had to paginate the second API request, I could have easily done so and flattened the results so that the end user still only sees a stream of Receipts.

So, we might do something like this, where our APIs will accept the normal API parameters, and an optional paging template. We will return a Stream of whatever the API provides which can then be consumed similarly to an Iterator.

pub fn get_something(arg1, arg2, paging: Option<PagingRequest>) -> impl Stream <Item=Something> { // ...
}

This way we still expose the pagination to the user but make the interface more forgivable. If they ever abandon the stream it will not continue endlessly doing requests since it is lazy :)

Dunno. Haven't thought too much about it, just ran into the problem of combining two potentially paging APIs in a friendly way.

[digorithm]

This is really cool @segfault-magnet, I love it. But, I believe we should implement and experiment with this on the events querying issue you're working on; it's a lot more likely the amount logs will be huge, and users will likely want to query it in near real-time, which justifies using streams and having this wonderful interface you came up with.

For the case of the current Provider APIs, I think we should keep it simple, for now, and avoid premature optimization. I'd say if the client's API takes in a PaginationRequest, the SDK wrapper API should also take it as well. cc @hal3e

[hal3e]

In the last commit, I have added a POC for the new provider API. The code is a bit more complicated than before but the UX is IMO much better. The idea was to give the user the possibility to do the pagination but still make the new API clean and easy to use.
As this is a POC I "converted" only two functions to get your opinion. If you like it I will convert the rest. Let's compare the old and new get_coins API. Previously we would return all coins associated to an address and there was no way to change it..

let coins = provider.get_coins(wallet.address(), asset_id).await?;

with the new API the call looks like this

let num_results = 2;
let coins = provider
    .get_coins_new(wallet.address(), asset_id, num_results)
    .call()
    .await?
    .results;

we use a builder pattern to create a ProviderPaginationCaller (I desperately need a better name 😄 ) and then we make and await the acctual call. We can also set the cursor, if we have one from a previous call, and we can also set the direction. Here is an example.

let paginated_result = provider
    .get_coins_new(wallet.address(), asset_id, 4)
    .call()
    .await?;
dbg!(paginated_result.results.len());

let paginated_result = provider
    .get_coins_new(wallet.address(), asset_id, 2)
    .with_cursor(paginated_result.cursor)
    .with_direction(PageDirection::Backward)
    .call()
    .await?;
dbg!(paginated_result.results.len());

Now about the code. If you look at the ProviderPaginationCaller struct you will see some complex types. The tricky part was to make the struct accept generic types and save the async function so that we can call it later with the call() method. As we do not know the size of the returned Futures we had to use dyn Future trait and use a Box. If anybody could make this code cleaner I would highly appreciate it 😄 . Here are the crazy types:

type BoxFnFuture<'a, T, U> = Box<dyn Fn(PaginationRequest<T>) -> BoxFutureResult<'a, U> + 'a>;
type BoxFutureResult<'a, U> = Box<dyn Future<Output = Result<U, ProviderError>> + 'a + Unpin>;

pub struct ProviderPaginationCaller<'a, T, U>
where
    T: Debug,
{
    /// The cursor returned from a previous query to indicate an offset
    pub cursor: Option<T>,
    /// The number of results to take
    pub results: usize,
    /// The direction of the query (e.g. asc, desc order).
    pub direction: PageDirection,
    // The function to call
    pub function: BoxFnFuture<'a, T, U>,
}

[digorithm]

Talking strictly related to the UX of this new API (and not related to the actual implementation), I believe the API has a nicer UX this way.

An important context to remember is how this would look like without @hal3e's proposal. We would need to always create a PaginationRequest; even if you're feeling lazy and don't want to paginate at all, which is most use cases. So it would look like this:

let pagination_request = PaginationRequest { // I have to import this
                        cursor: None, // Always passing None until I'm actually paginating
                        results: 100,
                        direction: PageDirection::Forward, // And also import this one;
                    };
                    
let coins = provider
    .get_coins_new(wallet.address(), asset_id, pagination_request) // Always need to pass a pagination request.
    .await?
    .results;

So, as we can see, we gain something by not having to .call(), but then we always have to create a PaginationRequest (or pass None if we were to change to accept an Option<PaginationRequest>).

With @hal3e's proposal, we drop the mandatory PaginationRequest, and whenever we need to paginate, we chain the methods with_cursor() and with_direction(). The downside is that we add a .call() to the workflow.

IMO, the .call() isn't too bad as it aligns with contract calls; all contract calls do have a .call() (or .simulate()). Kinda sounds like whenever you're calling something on-chain, you append the .call() to make it explicit, which I think is kinda cool :).

Let me know your thoughts @FuelLabs/sdk-rust.

[segfault-magnet]

Love what was done.

Perhaps this can evolve further into call accepting a &mut self and preparing the struct for getting the next page so that one might exhaust the endpoint without having to create a bunch of pagination structs.

[segfault-magnet]

Due to the way the closure is written the returned ProviderPaginationCaller must live as long as this provider on which the get_coins_new is being called.

This is a bit weird since we're cloning the provider regardless, why would it need to live as long as the original one?

A consequence of the fact that we're capturing &self inside the closure. Had we done something like this:

    pub fn get_coins_new(
        &self,
        from: &Bech32Address,
        asset_id: &AssetId,
        num_results: usize,
    ) -> ProviderPaginationCaller<String, PaginatedResult<Coin, String>> {
        let hash = Arc::new(from.hash().to_string());
        let asset_id = Arc::new(asset_id.to_string());
        let provider = Arc::new(self.clone());
        ProviderPaginationCaller::new(num_results, move |pr: PaginationRequest<String>| {
            let hash = Arc::clone(&hash);
            let asset_id_string = Arc::clone(&asset_id);
            let provider_clone = Arc::clone(&provider);
            Box::pin(async move {
                provider_clone
                    .client
                    .coins(&hash, Some(&asset_id_string), pr)
                    .await
                    .map_err(Into::into)
            })
        })

Then the ProviderPaginationCaller would have resolved 'static as its lifetime parameter 'a' and thus be able to live longer than the provider it was called on.

'static would be resolved because the returned closure moved the Arcs and thus became an owner of them of while previously it had first captured them by reference and then proceeded to clone them for the async block. Thus forcing the closure and, by extension, the returned ProviderPaginationCaller to have to live as long as self.

Since this is Fn and not FnOnce we have this pattern of Arc s being created outside the closure, then being moved inside the closure but the closure itself cannot give them to the async block because that would make the closure FnOnce since it had lost what it originally captured -- rather it has to clone the Arcs. Which is what arcs/rcs are for after all.

You can use Rcs as well if you decide to use LocalBoxFuture for some reason.

If you don't decide to implement the 'reuse ProviderPaginationCaller suggestion I made then you can relax this to FnOnce and simplify this capture process.

[hal3e]

@MujkicA and @leviathanbeak were able to reproduce the inconsistent behavior when getting paginated results from the client. @leviathanbeak will investigate it from the client side.