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

Access key allowance get charged for max allowed gas for tx instead what was spent #2523

Closed
vgrichina opened this issue Apr 24, 2020 · 1 comment · Fixed by #2742
Closed

Access key allowance get charged for max allowed gas for tx instead what was spent #2523

vgrichina opened this issue Apr 24, 2020 · 1 comment · Fixed by #2742
Assignees
@evgenykuzyakov
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)

Comments

@vgrichina
Copy link
Collaborator

Describe the bug
Access key allowance get charged for max allowed gas for tx instead what was spent.

My understanding (according to @evgenykuzyakov's investigation) is that we first charge attached gas per tx and then unspent gas gets refunded. It gets refunded for account, but not for access key allowance. This causes access key to run out of gas really quick.

To Reproduce

Try using any example app for a while with default gas settings from near-api-js.

Expected behavior
Access key allowance gets refunded for unspent gas.
@vgrichina vgrichina mentioned this issue Apr 24, 2020
Closed
5 tasks
@evgenykuzyakov evgenykuzyakov added the A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) label Apr 28, 2020
@ilblackdragon
Copy link
Member

From @evgenykuzyakov :
Basically we don't distinguish deposit refunds and gas refunds. So when a refund comes, we can't know whether to refund the access key or not. We may have a hack to include signer access key only to gas refunds.

@evgenykuzyakov evgenykuzyakov mentioned this issue May 19, 2020
Closed
@evgenykuzyakov evgenykuzyakov mentioned this issue May 28, 2020
Merged
1 task
nearprotocol-bulldozer bot pushed a commit that referenced this issue Jun 2, 2020
feat(Runtime): Refund allowance to the access keys (#2742)
be20d99 
Fixes: #2523

This PR introduces access keys allowance refunds using the old Receipt refund data structure by reusing existing fields.

Right now the refund receipt is identified by a receipt with the `predecessor_id == "system"`. These are special type of receipts that don't cost any gas and also don't generate refunds themselves.

This change reuses existing fields from the `ActionReceipt` such as `signer_id` and `signer_public_key` to distinguish between Gas Refunds and Deposit Refunds.
Gas refunds are refunds for the unused amount of Gas, while the Deposit Refunds are for the deposit amounts attached to transfer and function_call actions as well as refund for an action of deleting the account.

New Gas Refund Receipts also have the `predecessor_id == "system"`, but `signer_id == receiver_id` (because gas refunds always sent to the signer). While Deposit refunds still have `signer_id == "system"`. This allows to distinguish between Gas vs Deposit refunds.

The Gas Refund also uses the original `signer_public_key` to be able to identify the access key needed to refund the unused gas potion of the allowance.

When the Gas refund receipt arrives to the account, the original access key that signed the transaction that spawned this receipt might have been changed (e.g. using delete_key/add_key). This means that in some situation the gas refund towards the allowance can't be made, so we use best effort to refund the allowance, even if the access key is different.

- [x] TODO: Spec update

Spec update: near/NEPs#75

## Test plan
- Modified the test that covers function call access key and verified that it generated the refund.
- CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@evgenykuzyakov evgenykuzyakov

Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(Runtime): Refund allowance to the access keys near/nearcore
3 participants
@vgrichina @ilblackdragon @evgenykuzyakov