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

Analyze usage of restricted keywords in composite fields on Mainnet #2998

Closed
j1010001 opened this issue Jan 9, 2024 · 10 comments · Fixed by #3057
Closed

Analyze usage of restricted keywords in composite fields on Mainnet #2998

j1010001 opened this issue Jan 9, 2024 · 10 comments · Fixed by #3057
Assignees

Comments

@j1010001
Copy link
Member

j1010001 commented Jan 9, 2024

As @bjartek pointed out, some resources might be using field names that will be restricted keywords in Cadence 1.0.
We need to analyze contracts deployed on Miannet to identify those instances so that we can with with the developer to migrate the on-chain data for those cases, if any exist.

@bjartek
Copy link
Contributor

bjartek commented Jan 9, 2024

It is safe to assume that the keywords in question are from runtime/parser/keyword.go? And these are not allowed as fields anymore?

@bjartek
Copy link
Contributor

bjartek commented Jan 9, 2024

reserved-report.json

i did a sql query with all instances of
\slet <keyword>\s or \svar <keyowrd>\s
for all the keywords in that file.

uploaded the results as json here. Note that some of these only use it in temp variables but the list is 252 long so it is a lot better then starting looking at 3500 contracts.

if you want me to tweak the sql here it is

SELECT identifier, body FROM contracts WHERE valid_to IS null AND 
(
body ~ '\mlet\sif\M' or
body ~ '\mlet\selse\M' or
body ~ '\mlet\swhile\M' or
body ~ '\mlet\sbreak\M' or
body ~ '\mlet\scontinue\M' or
body ~ '\mlet\sreturn\M' or
body ~ '\mlet\strue\M' or
body ~ '\mlet\sfalse\M' or
body ~ '\mlet\snil\M' or
body ~ '\mlet\slet\M' or
body ~ '\mlet\svar\M' or
body ~ '\mlet\sfun\M' or
body ~ '\mlet\sas\M' or
body ~ '\mlet\screate\M' or
body ~ '\mlet\sdestroy\M' or
body ~ '\mlet\sfor\M' or
body ~ '\mlet\sin\M' or
body ~ '\mlet\semit\M' or
body ~ '\mlet\sauth\M' or
body ~ '\mlet\saccess\M' or
body ~ '\mlet\sall\M' or
body ~ '\mlet\sself\M' or
body ~ '\mlet\sinit\M' or
body ~ '\mlet\scontract\M' or
body ~ '\mlet\saccount\M' or
body ~ '\mlet\simport\M' or
body ~ '\mlet\sfrom\M' or
body ~ '\mlet\spre\M' or
body ~ '\mlet\spost\M' or
body ~ '\mlet\sevent\M' or
body ~ '\mlet\sstruct\M' or
body ~ '\mlet\sresource\M' or
body ~ '\mlet\sinterface\M' or
body ~ '\mlet\sentitlement\M' or
body ~ '\mlet\smapping\M' or
body ~ '\mlet\stransaction\M' or
body ~ '\mlet\sprepare\M' or
body ~ '\mlet\sexecute\M' or
body ~ '\mlet\scase\M' or
body ~ '\mlet\sswitch\M' or
body ~ '\mlet\sdefault\M' or
body ~ '\mlet\senum\M' or
body ~ '\mlet\sview\M' or
body ~ '\mlet\sattachment\M' or
body ~ '\mlet\sattach\M' or
body ~ '\mlet\sremove\M' or
body ~ '\mlet\sto\M' or
body ~ '\mlet\srequire\M' or
body ~ '\mlet\sstatic\M' or
body ~ '\mlet\snative\M' or
body ~ '\mlet\spub\M' or
body ~ '\mlet\spriv\M' or
body ~ '\mlet\sinclude\M' or
body ~ '\mvar\sif\M' or
body ~ '\mvar\selse\M' or
body ~ '\mvar\swhile\M' or
body ~ '\mvar\sbreak\M' or
body ~ '\mvar\scontinue\M' or
body ~ '\mvar\sreturn\M' or
body ~ '\mvar\strue\M' or
body ~ '\mvar\sfalse\M' or
body ~ '\mvar\snil\M' or
body ~ '\mvar\slet\M' or
body ~ '\mvar\svar\M' or
body ~ '\mvar\sfun\M' or
body ~ '\mvar\sas\M' or
body ~ '\mvar\screate\M' or
body ~ '\mvar\sdestroy\M' or
body ~ '\mvar\sfor\M' or
body ~ '\mvar\sin\M' or
body ~ '\mvar\semit\M' or
body ~ '\mvar\sauth\M' or
body ~ '\mvar\saccess\M' or
body ~ '\mvar\sall\M' or
body ~ '\mvar\sself\M' or
body ~ '\mvar\sinit\M' or
body ~ '\mvar\scontract\M' or
body ~ '\mvar\saccount\M' or
body ~ '\mvar\simport\M' or
body ~ '\mvar\sfrom\M' or
body ~ '\mvar\spre\M' or
body ~ '\mvar\spost\M' or
body ~ '\mvar\sevent\M' or
body ~ '\mvar\sstruct\M' or
body ~ '\mvar\sresource\M' or
body ~ '\mvar\sinterface\M' or
body ~ '\mvar\sentitlement\M' or
body ~ '\mvar\smapping\M' or
body ~ '\mvar\stransaction\M' or
body ~ '\mvar\sprepare\M' or
body ~ '\mvar\sexecute\M' or
body ~ '\mvar\scase\M' or
body ~ '\mvar\sswitch\M' or
body ~ '\mvar\sdefault\M' or
body ~ '\mvar\senum\M' or
body ~ '\mvar\sview\M' or
body ~ '\mvar\sattachment\M' or
body ~ '\mvar\sattach\M' or
body ~ '\mvar\sremove\M' or
body ~ '\mvar\sto\M' or
body ~ '\mvar\srequire\M' or
body ~ '\mvar\sstatic\M' or
body ~ '\mvar\snative\M' or
body ~ '\mvar\spub\M' or
body ~ '\mvar\spriv\M' or
body ~ '\mvar\sinclude\M' 
) 

@bjartek
Copy link
Contributor

bjartek commented Jan 10, 2024

This query was run against mainnet on current block height when i run it.

@turbolent
Copy link
Member

Thank you Bjarte!

I'll write an analyzer for the linter to only find fields

@turbolent
Copy link
Member

I used #3022 to query all MN contracts from 2023-11-16 with the hard keywords of #3019, and results are:

  • event: 4
  • contract: 9
  • default: 1

That means there are currently some contracts which are effected, but given there are thousands of contracts, the results are pretty OK, and we can also do something about it.

We already define some keywords to be soft keywords instead of hard keywords, i.e. they may be used as identifiers: https://github.com/onflow/cadence/pull/3019/files#diff-d4f2e1652a85c54fe0afb9d6c1ae50164d96d7051160e201669e9d6699e5bc2eR169. It seems reasonable to extend this list.

We could also consider allowing more keywords specifically for field identifiers. For example, while a declaration event event() seems odd and should be rejected, a field declaration let event: T seems reasonable.

@turbolent turbolent removed their assignment Jan 17, 2024
@turbolent
Copy link
Member

@SupunS @dsainati1 Could one of you maybe take this over and make the changes needed? It would allow me to help Faye with the atree inlining work

@SupunS
Copy link
Member

SupunS commented Jan 19, 2024

I can have a look

@SupunS
Copy link
Member

SupunS commented Jan 29, 2024

So apparently, hard keywords are restricted only in certain places (i.e: places where ambiguity can be created, such as composite declaration names, function names, etc.). And they are not restricted for fields, and many other places.
So these use-cases actually work even now. Added a test: #3057

@SupunS
Copy link
Member

SupunS commented Jan 29, 2024

That was a bit of a surprise, I was under the impression that hard keywords are rejected everywhere, while soft keywords are allowed on certain places.
But realized it is slightly different from what I was assuming: hard keywords are allowed in certain places, while soft keywords are allowed everywhere.

@turbolent
Copy link
Member

That's actually great to see! Could you also please add a comment to the code defining the hard and soft keywords explaining this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants