8000 feat: stabilize account_id_in_function_call_permission protocol feature by matklad · Pull Request #7569 · near/nearcore · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: stabilize account_id_in_function_call_permission protocol feature #7569

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

Merged
merged 3 commits into from
Sep 7, 2022

Conversation

matklad
Copy link
Contributor
@matklad matklad commented Sep 6, 2022

account_id_in_function_call_permission

This feature controls the check which enforces that account id in function call permission is indeed a valid account id. Before, any string could have been used there. The primary motivation is robustness -- by restricting permissions to only valid account ids, we don't have to deal with potentially arbitrary long strings in storage.

Context

Testing and QA

We have basic and upgradability test here. This PR also adds a test for an extra edge case with overly long account id. This feature have been running on betanet for couple of months without problems.

Checklist

@matklad matklad requested a review from a team as a code owner September 6, 2022 17:57
@matklad matklad force-pushed the m/stabilize-valid-account-id branch 2 times, most recently from 53e7c04 to 925392a Compare September 6, 2022 18:11
@matklad matklad force-pushed the m/stabilize-valid-account-id branch from 925392a to 7707ef8 Compare September 6, 2022 18:20
@matklad matklad requested a review from akhi3030 September 7, 2022 09:48
@matklad
Copy link
Contributor Author
matklad commented Sep 7, 2022

@akhi3030 could you also take a look here? I think we require two approvals for stabilization?

@akhi3030
Copy link
Collaborator
akhi3030 commented Sep 7, 2022

Took me some time to figure out where we are setting account_id_validity_rules_version to AccountIdValidityRulesVersion::V1. It is being done in https://github.com/near/nearcore/pull/7569/files#diff-b342c1a9a2edce343ac58ac9824f9dc63fee236d0e882fe0752930990bf1e684.

My only other question is do we know if this impacts any existing workflows? If this breaks some existing workflow, do we have a plan on how to address that?

@matklad
Copy link
Contributor Author
matklad commented Sep 7, 2022

I don't think this would impact existing valid workflows:

  • existing malformed keys in the DB continue to not do anything
  • submitting a new malformed key would now return an error, but, given that malformed keys are essentially no-ops, I don't think this would be disruptive, more like revealing some existing problem (spending tokens on adding useless keys). I don't actually know if there are any transactions with invalid acount ids in the wild, but my value judgement is that it doesn't make sense to spend time figuring that out.

@near-bulldozer near-bulldozer bot merged commit 85f1479 i 8000 nto near:master Sep 7, 2022
nikurt pushed a commit that referenced this pull request Sep 7, 2022
…re (#7569)

# account_id_in_function_call_permission

This feature controls [the check](https://github.com/near/nearcore/blob/b315192e84d388671316deaa3a17ece9d0565fd1/runtime/runtime/src/verifier.rs#L400-L405) which enforces that account id in function call permission is indeed a valid account id. Before, any string could have been used there. The primary motivation is robustness -- by restricting permissions to only valid account ids, we don't have to deal with potentially arbitrary long strings in storage. 

# Context

- Implementation: #7139

# Testing and QA

We have basic and upgradability test [here](https://github.com/near/nearcore/blob/master/integration-tests/src/tests/client/features/account_id_in_function_call_permission.rs#L18). This PR also adds a test for an extra edge case with overly long account id. This feature have been running on betanet for couple of months without problems. 

# Checklist
- [x] Link to nightly nayduck run: https://nayduck.near.org/#/run/2667
- [x] Update CHANGELOG.md to include this protocol feature in the `Unreleased` section.
nikurt pushed a commit that referenced this pull request Nov 9, 2022
…re (#7569)

# account_id_in_function_call_permission

This feature controls [the check](https://github.com/near/nearcore/blob/b315192e84d388671316deaa3a17ece9d0565fd1/runtime/runtime/src/verifier.rs#L400-L405) which enforces that account id in function call permission is indeed a valid account id. Before, any string could have been used there. The primary motivation is robustness -- by restricting permissions to only valid account ids, we don't have to deal with potentially arbitrary long strings in storage. 

# Context

- Implementation: #7139

# Testing and QA

We have basic and upgradability test [here](https://github.com/near/nearcore/blob/master/integration-tests/src/tests/client/features/account_id_in_function_call_permission.rs#L18). This PR also adds a test for an extra edge case with overly long account id. This feature have been running on betanet for couple of months without problems. 

# Checklist
- [x] Link to nightly nayduck run: https://nayduck.near.org/#/run/2667
- [x] Update CHANGELOG.md to include this protocol feature in the `Unreleased` section.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0