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.

Sign up for GitHub

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

Jump to bottom

Exposing Function Call Access Key Info At Runtime #371

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Exposing Function Call Access Key Info At Runtime #371

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1387a2c
initial impl of new NEP for access keys
BenKurrek Jul 13, 2022
c99a32d
renamed NEP version to match PR
BenKurrek Jul 13, 2022
3aadb58
Changed NEP to only focus on function call access keys
BenKurrek Jul 14, 2022
047873d
Add runtime specification
jakmeier Jul 15, 2022
7e52d6c
Exact gas cost specification
jakmeier Jul 15, 2022
186f024
Changed NEP to review and added jakob's info
BenKurrek Jul 15, 2022
aecf5ad
Add gas costs to write to registers
jakmeier Jul 18, 2022
74f2447
Be more specific about input public key
jakmeier Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 170 additions & 0 deletions neps/nep-0371.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
---
NEP: 371
Title: Runtime Function Call Access Key Info
Author: Ben Kurrek <ben.kurrek@near.org>, Jakob Meier <jakob@near.org>
DiscussionsTo: https://gov.near.org/t/expose-gas-price-and-contract-access-key-info-within-runtime/24788
Status: Draft
Type: Standards Track
Category: Review
Created: 13-Jul-2022
---

## Summary

There is currently no way to query for information about access keys stored on a contract at runtime. Adding the host function `access_key_info` to the runtime specification will allow to query this for the current account an easy way. This fully covers the most important use cases that are currently known for querying access keys at runtime.

## Motivation

As contracts start to use access keys in more creative ways, developers will want to know information such as the access key's allowance, receiver and more. Since contracts are often built for scalability, Gas and storage costs are often delegated to the end user. If you charge the user for an access key and its allowance, when the key is deleted or used up, that allowance should be refunded to the purchaser. Currently, there is no way to query for the leftover allowance before a key is deleted. This information can tell you how much Gas was spent, and allows for a slew of different scenarios to arise.
akhi3030 marked this conversation as resolved.
Show resolved Hide resolved

## Rationale and alternatives

Without creating a promise, we can only query for information stored on the same shard. This is why in this proposal, the access key information at runtime will only come from keys stored on the current contract. We had also investigated the possibility of introducing pagination for all keys stored on the contract but this is infeasible at the moment due to the significant redesign requirement of the VM logic.

## Runtime specification

The contract runtime provides one host function called `access_key_info` which is callable in regular calls and views.

```rust
fn access_key_info (
Copy link
Contributor

Choose a reason for hiding this comment

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

So this API can only be used to access info about a known key? What if a contract wants to know the actual set of keys associated with it? Maybe there is already an API for that or is that not an interesting use case?

Copy link
Contributor

Choose a reason for hiding this comment

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

It could interesting to paginate keys, we just haven't had anyone coming up with an actual use case, yet. :)

And because the API for that would be significantly trickier to implement, we opted it out, for now.

I expect most calls to either look up the current access key (signer_account_pk() already exists for that) or otherwise take an argument from outside that specifies which access keys to check. Listing all keys through rpc REST API has been supported for a long time, so the signer side should always be able to provide the list if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

And because the API for that would be significantly trickier to implement

To expand on this, we don't really have a way to iterate keys in our runtime.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I understand this API now. Given a pk, it allows a contract to look up various details about it. How the contract got hold of the pk is outside of its scope. That makes sense.

Thinking out loud. If we expect a lot of use cases to be of the style: access_key_info(signer_account_pk(), ...), would it make sense to (in future) offer an optimising API here: access_key_for_signer(). This will mean that we save two copies of the public key. But I suppose that I am premature optimising here. And we can always introduce this API in the future if we think it is warranted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's important to note that the PK that can be passed in MUST belong to the contract account. If benji signs a txn with an access key where the contract is the receive, this won't return any info. The signer's account must be the contract.

/// in: number of bytes of the public key to query OR u64::MAX
public_key_len: u64,
/// in: pointer OR register ID to the public key to query
public_key_ptr: u64,
/// out: pointer to where a u64 can be written by the host
nonce_ptr: u64
/// out: pointer to where a u128 can be written by the host
allowance_ptr: u64,
/// out: register to write the access key holder account id
account_id_register_id: u64,
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should guarantee that this'll be a valid AccountId? Today we have some keys where the receiver_id is borked and isn't actually an AccountId. We are fixing that at the runtime level: near/nearcore#7139.

More specifically, if this fn queries for a key whose receiver isn't an AccountId, we can:

  • return receiver as a string
  • or pretend that the key doesn't actually exist.

Copy link
Contributor

@jakmeier jakmeier Jul 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good point. My gut feeling here says, if the access key exists, correct or not, this function should be able to reveal this. Otherwise code that checks if a key already exists before adding a new key like this will be buggy:

//...
signer_account_pk(pk_register_id);
let result = access_key_info (
    u64::MAX,
    pk_register_id,
    &mut nonce as *mut u64 as _,  
    &mut allowance as *mut u128 as _, 
    account_register_id,
    method_register_id,
);
match result {
 0 => {
       // key does not exist, so deploying should be possible, right?
       signer_account_id(signer_register_id);
       promise_batch_action_add_key_with_full_access(promise_idx, u64::MAX, signer_register_id, 0);
       // oops, above promise can fail with `AddKeyAlreadyExists` :(
    },
1 | 2 => {
        // key exists, need to delete it first
       promise_batch_action_delete_key(promise_idx, u64::MAX, pk_register_id);
       signer_account_id(signer_register_id);
       // now the new key can  be added
       promise_batch_action_add_key_with_full_access(promise_idx, u64::MAX, signer_register_id, 0);
    }
}

Therefore, I tend towards returning a string and making it clear that it may not necessarily reflect a valid account id.
We also return non-existing but valid receiver account ids. And the returned id would only be invalid if the user (or someone else with full access) has previously put the wrong id there. So this extra caveat seems like a non-issue from a user's perspective.

/// out: register to write the method names as a comma separated list
Copy link
Contributor

Choose a reason for hiding this comment

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

as a comma separated list

This is problematic: , is a valid character in a method name, so this API creates possibility for ambiguity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wow, I didn't realise it is technically an allowed character in a WASM method name. Very glad you point this out.

But that's interesting. When adding a function access key from within a contract, then the encoding used is a comma separated list, so this makes it impossible to create such a method access key this way. But that's just an artificial limitation from the host function implementation. From near CLI, it is possible to list methods with a , in it. And there is also nothing preventing for a contract with such a method name to be deployed. So this is a real concern.

method_names_register_id: u64,
)
-> u64;
```

Just like other host functions, such as `storage_read`, if `public_key_len` is set to `u64::MAX` it will treat the input as a register input instead of memory input.
That is, `public_key_ptr` will be used as a register id.

### Return values

- Returns 0 if the given public key is not registered as access key to the current account. In this case, the memory locations behind `nonce_ptr` and `allowance_ptr` are not changed by the host. The register content of `account_id_register_id` and `method_names_register_id` are not changed, either.
- Returns 1 if the given public key is a full access key to the current account. The memory location at `nonce_ptr` contains the current nonce value of the access key. The memory location at `allowance_ptr` is not changed by the host. The register content of `account_id_register_id` and `method_names_register_id` are not changed, either.
- Returns 2 if the given public key is a function call access key to the current account. The memory location at `nonce_ptr` contains the current nonce value of the access key. The memory location at `allowance_ptr` contains a `u128` value that is the remaining allowance in yoctoNEAR. The register `account_id_register_id` contains the account name of the access key holder. The register `method_names_register_id` contains a comma separated list of all method names that the access key is valid for.

### Panics

- `InvalidPublicKey`: Calling `access_key_info` with an invalid public key.
akhi3030 marked this conversation as resolved.
Show resolved Hide resolved
- `MemoryAccessViolation`: Any of the following ranges are not fully inside the guest memory.
- [public_key_ptr, public_key_ptr + public_key_len)
- [nonce_ptr, nonce_ptr + 8)
- [allowance_ptr, allowance_ptr + 16)
- `MemoryAccessViolation`: If writing to the registers increases the total register memory beyond `registers_memory_limit`.
- `InvalidRegisterId`: `account_id_register_id` and `method_names_register_id` are equal

### Gas cost

No new gas cost parameter needs to be created. The workload put upon the runtime is almost exactly the same as `storage_read`, with slight changes to how the key is constructed and how values are returned

The gas cost for accessing is the base host-function cost, storage read costs including trie node costs, and writing to guest memory cost. The exact list is given here.

- `wasm_base` *-- host-function call base cost*
- `wasm_storage_read_base`
- `wasm_storage_read_key_byte` * `(2 + public key length + number of bytes in current account id)`
- `wasm_storage_read_value_byte` * `(number of bytes in method_names + number of bytes in returned account id)`
- `wasm_touching_trie_node` * `(number of accessed trie nodes that were no in chunk cache)`
- `wasm_read_cached_trie_node` * `(number of accessed trie nodes that were in chunk cache)`
- `wasm_write_memory_base` *-- for writing nonce to guest memory*
- 8 * `wasm_write_memory_byte` *-- for writing nonce to guest memory*

If input public key is read from memory:
- `wasm_read_memory_base`
- `public_key_len` * `wasm_read_memory_byte`

If input public key is read from register (`public_key_len` == `u64::MAX`):
- `wasm_read_register_base`
- (public key length) * `wasm_read_register_byte`

Additional costs that only occur if the result is a function access key:
- `wasm_write_memory_base` *-- for writing allowance*
- 16 * `wasm_write_memory_byte` *-- for writing allowance*
- 2 * `write_register_base`
- `wasm_write_register_byte` * `(length of returned account id)`
- `wasm_write_register_byte` * `(length of returned method names list)`

### Rationale behind runtime specification choices

The parameter values are mostly analog to `promise_batch_action_add_key_with_function_call` but only the public key is an input parameter. All other parameters are used to define output locations.

The output for nonce and allowance is of a small and fixed size, so a pointer to guest memory works well.

The Receiver ID can be between 2 and 64 bytes, based on the limits of account id length. It is also thinkable that this range changes in the future. Using a register to return the location is the most flexible solution to make the host-function signature agnostic to this dynamic size.

The method names list can be between 0 and 2000 bytes, based on `max_number_bytes_method_names` runtime parameter. Again, it is thinkable that this range changes in the future. The register is again the right choice here.

## User perspective

In the runtime, access key information will be queryable via a function by passing in the desired public key. The key information will be of the following form.

```js
enum KeyPermission {
FullAccess,
FunctionCall(FunctionCall),
}

type KeyInformation = {
public_key: string,
access_key: {
nonce: number,
permission: KeyPermission
}
}

type FunctionCall = {
receiver_id: string,
allowance: string,
method_names: Array<string>,
}
```

This information should be queryable via a new function called `access_key_info_for_current_contract` which takes a public key as a parameter.

```ts
function access_key_info_for_current_contract(
public_key: &PublicKey,
): KeyInformation | null
```

This function should be exposed in the environment and callable using `env::access_key_info_for_current_contract();`. An example of this can be seen below.

```rs
pub fn check_key_exists(&self, pk: PublicKey) {
let key_info: FunctionCallKeyInfo = env::access_key_info_for_current_contract(&pk);

if let Some(info) = key_info {
near_sdk::log!("key info found");
} else {
near_sdk::log!("No public key found");
}
}
```

An example of returning the allowance of the key can be seen below.

```rs
pub fn check_key_allowance(&self, pk: PublicKey) {
let key_info: FunctionCallKeyInfo = env::access_key_info_for_current_contract(&pk).unwrap();

let allowance = fc.allowance;
near_sdk::log!("key allowance: {}", allowance);
}
```

## Future possibilities

Some of the future possibilities for the standard could be to return a vector of KeyInformation by paginating through the keys on the contract. In addition, we could add a `key_total_supply` function that returns the number of access keys stored on the contract. One could also expand and introduce the ability to query for access key info from other contracts by using a promise.

## Copyright
[copyright]: #copyright

Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).