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.

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

Explain why FunctionCallPersmission doesn't use AccountId #4739

Closed
matklad opened this issue Aug 26, 2021 · 1 comment · Fixed by #4773
Closed

Explain why FunctionCallPersmission doesn't use AccountId #4739

matklad opened this issue Aug 26, 2021 · 1 comment · Fixed by #4773
Assignees
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)

Comments

@matklad
Copy link
Contributor

matklad commented Aug 26, 2021

This looks suspect to me:

/// The access key only allows transactions with the given receiver's account id.
pub receiver_id: String,

I'd expect this to be AccountId. This was introduced in 2d7b3c0#diff-e15377fa8f9e6deba5e3549c4b497cb0c96b0d799982a30ec5d6ef442729d32aR203, so I suspect the reason is that "it'd be a protocol change" is the reason for that.

This needs a comment in the source code I think, cc @miraclx

@miraclx miraclx self-assigned this Aug 26, 2021
@miraclx
Copy link
Contributor

miraclx commented Aug 26, 2021

Not necessarily because of a protocol change, it's caused by the testnet's genesis config containing invalid AccountId-s in that position which caused canary nodes to crash [See fix in #4621 (comment)].
But, I agree, an explanatory comment is in order.

@bowenwang1996 bowenwang1996 added the A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) label Aug 26, 2021
near-bulldozer bot pushed a commit that referenced this issue Sep 6, 2021
…_id != AccountId (#4773)

Resolves #4739: documents why the `receiver_id` field in `FunctionCallPermission` isn't an `AccountId` despite recently refactoring the whole workspace to using strict `AccountId`-s.

See #4621 (comment) for more context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc)
Projects
None yet
3 participants