Utilize ABI for improved ergonomics

[ChaoticTempest]

With the imminent release of SDK 4.1 with ABI support, it would be great if we can improve the ergonomics of calling into contract functions, with the addition of a way to supply the ABI:

#[path = "../gen/status_msg.rs"]
mod status_msg;

...
let contract: AbiClient = worker.dev_deploy("status_message.wasm")
   .with_abi::<status_msg::AbiClient>()
   .await?;

contract.set_status("account_id", "message").await?;
let msg: String = contract.get_status("account_id").await?;

I'm not sure what the embedded ABI version would look like though. Is there currently some way to grab the resulting rust types from the wasm file?

@austinabell @itegulov @miraclx WDYT?

[austinabell]

Linking this repo as it will be relevant for others https://github.com/near/near-abi-client-rs

As for supplying the interface through a generic, that could be interesting, but I don't know how you would go about attaching the functions on the contract object to call directly.

Also, feels like it would be less maintenance lift to just cut out the middle step of generating the abi file and maybe have some proc macro or build script that takes in the project root rather than the abi file?

[miraclx]

Yeah, this sounds reasonable to me. Just a couple of notes on things that immediately jump out at me from the current API.

1. We can re-export near-abi-client's exports, so users don't need to explicitly include that in their dependency list.
2. The client doesn't need to own the Contract. It just needs a reference to it. (ref). This inadvertently means it can't be a single batched line for deploying and client construction.
3. Contract::batch would be sorely missed with this interface. But that should be a simple add. Although now, the question is, should we only include call functions behind this batch API?
4. Also, since workspaces::rpc::client::{DEFAULT_CALL_DEPOSIT, DEFAULT_CALL_FN_GAS} are not exported, the current code generated ABI clients forces users to define these values.
   I think for view functions, it's perfectly fine to finalize the request. But for call functions, we should just return some struct, similar to a CallTransaction that is pre-set with default values for gas and deposit, which you can optionally set. After which you have to manually call .transact().await.
5. Also, I think it's important to recognize the distinction that the AbiClient is a client for the signer (more often than not, this is a user). Something like dev_deploy returns a Contract who's signer is the same as the deployer.
6. This means, client construction for already deployed contracts is weird too. Because you would have to construct a Contract instance, which doesn't disambiguate contract_id from signer_id.

Given all this, I think the best way forward is to change the interface of the code generated AbiClients, and instead of exposing the contract methods directly, we expose two methods – view and call.

pub struct AbiClient {
    contract_id: AccountId,
}

impl AbiClient {
    pub fn new(contract_id: AccountId) -> Self;

    pub fn id(&self) -> &AccountId;
    pub fn view(&self) -> ViewMethod;
    pub fn call(&self, signer: &workspaces::Account) -> CallMethod;
}

pub struct ViewMethod<'a> {
    client: &'a AbiClient,
}

impl<'a> ViewMethod<'a> {
    pub async fn get_status(&self) -> Result<ViewResultDetails>;
}

pub struct CallMethod<'a> {
    client: &'a AbiClient,
    actions: Vec<Action>,
    gas: Gas,
    deposit: Deposit,
}

// batchable by default
impl<'a> CallMethod<'a> {
    pub fn set_status(self, message: String) -> Self;
}

// small nit here, potential identifier conflict
impl<'a> CallMethod<'a> {
    pub fn gas(self, gas: Gas) -> Self;
    pub fn deposit(self, deposit: Deposit) -> Self;
    pub async fn transact(self) -> Result<ExecutionFinalResult>;
}

And for the consumer of this API:

pub mod status_msg {
    use workspaces::abi;

    abi::generate!(Client for "abis/status_message.json");
}

let worker = workspaces::sandbox().await?;

let contract = worker.dev_deploy(include_bytes!("status_message.wasm")).await?;
// here, env::signer_account_id() == env::current_account_id()

let client = status_msg::Client::new(contract.id().clone());

let signer_id = "miraclx.near".parse::<AccountId>()?;
let signer = Account::from_secret_key(signer_id, SK, &worker);

// call functions
client
    .call(&signer)
    .set_status("testing...".to_owned())
    .set_status("I'm a teapot ;-)".to_owned())
    .gas(parse_gas!("1 Tgas"))
    .deposit(parse_near!("1 N"))
    .transact()
    .await?;

// view functions
assert!(
    client.view().get_status(account.id()).await?,
    "I'm a teapot ;-)"
);

The only nit is the potential conflict between the identifiers gas, deposit, transact and the ones that were code generated. But I'm exploring a potential resolution for this.

[itegulov]

I'm not sure what the embedded ABI version would look like though. Is there currently some way to grab the resulting rust types from the wasm file?

I don't think there is anything ready, but shouldn't be too difficult to implement.

Also, feels like it would be less maintenance lift to just cut out the middle step of generating the abi file and maybe have some proc macro or build script that takes in the project root rather than the abi file?

I think both options have the right to exist. Suppose I don't have access to the contract's source code, but I have a wasm file with embedded ABI.

[ChaoticTempest]

@miraclx I feel like all that might just introduce too many ways to call into the same thing.

A little bit more verbose than the initial syntax I suggested, but we can continue to use purely account.{call, batch} where something like abi_status_msg.set_status("some message") simply returns a workspaces::Function value. And then we'd get the very consistent API of:

let status_msg: workspaces::AbiContract<status_msg::AbiClient> = 
    worker.dev_deploy("status_message.wasm")
       .with_abi::<status_msg::AbiClient>()
       .await?;
let method: Function = status_msg.set_status("some message");

account.call(method)
   .gas(...)
   .deposit(...)
   .transact()
   .await?;

account.batch()
   .call(method)
   .call(other_method)
   .transact()
   .await?;

Which wouldn't require you guys to change too much of the ABI generation code if at all.

[miraclx]

Hm, that's a neat alternative. Although, neither Account::call nor Account::view take in Functions. Only the account.batch().call flow is consistent with the current API.

Also, Function already has gas and deposit methods, so it will be more like:

let method: Function = status_msg
    .set_status("some message")
    .gas(...)
    .deposit(...);

But, Function as it's currently defined is a builder, so returning that from the client is a bit weird. Nothing stops the caller from calling .args{,_json} manually and overwriting the pre-serialized args. Instead, I'd expect what would be most useful in this case would've been inspecting the serialized args before sending the method off. Ergo: method.args() -> &[u8] instead of method.args(self, ...) -> Self.

I certainly like this interface, as it cleans things up a bit, and also solves the nit (potential identifier conflict) in my first proposal.

But I'm not sure the types (Account, Function) as currently defined would be ideal for this.

---

Another unresolved point is using Contract::with_abi only helps if you already have a Contract instance. Which you can only get if you manually constructed one, but constructing one requires you to specify a SecretKey. Which is unrelated to the scope if you only want to make view calls, or even if you want to submit transactions you'd have to create a random disposable secret key just so you can create your Contract. And despite that key being unused, as a user you'd be directly exposed to it, and probably think it does something. Making it even easier to confuse with their own keys.

[ChaoticTempest]

Although, neither Account::call nor Account::view take in Functions

Definitely taking a little liberty with the current API, which I should've mentioned haha. But probably something like the following signature is what is more realistic:

Account::{call, view}(AccountId, Into<Function>)

which shouldn't break any of the current usage since we can have Into<Function> for &str.

Also, Function already has gas and deposit methods, so it will be more like:

let method: Function = status_msg
   .set_status("some message")
   .gas(...)
   .deposit(...);

But, Function as it's currently defined is a builder, so returning that from the client is a bit weird. Nothing stops the caller from calling .args{,_json} manually and overwriting the pre-serialized args. Instead, I'd expect what would be most useful in this case would've been inspecting the serialized args before sending the method off. Ergo: method.args() -> &[u8] instead of method.args(self, ...) -> Self.

With the above signature of Into<Function>, we don't necessarily have to go with supplying gas and deposit methods being there as contract.set_status can still proceed to return whatever Abi object. But also, in workspaces currently, there's really nothing stopping people from supplying args* functions multiple times anyways, with the proceeding one overwriting the previous call.

Another unresolved point is using Contract::with_abi only helps if you already have a Contract instance. Which you can only get if you manually constructed one, but constructing one requires you to specify a SecretKey. Which is unrelated to the scope if you only want to make view calls, or even if you want to submit transactions you'd have to create a random disposable secret key just so you can create your Contract. And despite that key being unused, as a user you'd be directly exposed to it, and probably think it does something. Making it even easier to confuse with their own keys.

I see, yeah then we probably need a different type for this kind of case. Maybe something like what we have current for Contract type is an owned contract where the user usually refers to when making deploys, but for purely an AbiClient kind of version, we can have a separate type something like ContractView