FLIP for removing pub, pub(set) and priv

[dsainati1]

One of the followup proposals from #54 is to remove the pub, pub(set), and priv access modifiers from Cadence as one potential way to alert developers that their code will need to be reworked to support the new entitlements paradigm.

[turbolent]

Thank you for opening this FLIP.

If approved, we should consider providing a code migration tool to assist developers in the migration. It could for example be part of the language server: provide a diagnostic that indicates an error or deprecation, and provide a quick fix that replaces e.g. priv with access(self)

[dsainati1]

I mentioned this option in the FLIP, and while I think it's worth considering, we should be careful with the migration too IMO. Swapping priv with access(self) would be fine, but not all pub modifiers should be replaced with access(all), as some should use new entitlement access. Part of the potential benefit of this change would be that it would require developers to consider individually each use of pub and what to replace it with.

[turbolent]

The description notes:

[...] potential way to alert developers that their code will need to be reworked to support the new entitlements paradigm.

This works in the case where those modifiers are used. We should still have a mechanism that handles e.g. access(all) – programs that use access(all) instead of pub already, should still be forced to be updated (maybe even as simple as, having to "redeploy" the same contract to acknowledge that no update is needed/wanted)

[joshuahannan]

yeah, I wouldn't want to do an automatic migration either, but the proposal makes sense

[dsainati1]

How common is it for access(all) to be used in practice currently?

[joshuahannan]

If I had to guess, and there is no actual data backing this up, I'd say about 20% of the access(all) declarations are access(all) and the other 80% are pub. That is just from what I've seen. I might be wrong though

[dsainati1]

That is a lot larger than I'd have guessed, and if it is indeed that high of a percentage we should have a plan to handling those contracts.

[turbolent]

👍

I like how this is going to simplify and make things more consistent (access(X)), as well as reduce the confusion around pub(set)

[bjartek]

I also like that it will make things simpler. One way is better then multiple ones imho. However I am concerned about migration.

[turbolent]

@bjartek Migration should be simple and straight forward here. It might be good to point that out in the FLIP:
Replace existing uses of pub with access(all), uses of priv with access(self), and uses of pub(set) with access(all) and add a setter function.

[turbolent]

@dsainati1 This FLIP has been open for a month and there has not been any additional feedback, let's accept and merge as discussed in the Language Design Meeting 🎉

[bz-hashtag-0780]

[EDIT: removed the image]

I guess this is a bit too late to raise any concerns around the change

after I was made aware of the change of removing pub i started writing all my contracts with access(all) i didn't really expect it to be a big deal so didn't want to comment on the flip, but 10 smart contracts later, i have developed a deep frustration with the change because it just takes so much longer to write contracts in cadence

going from pub to access(all) takes a lot of effort when you consider you have to do it 20-200 times on each contract which is 180-1800 more characters per contract, cadence is cool because it's meant for rapid iteration, experimentation and being developer-friendly, this change kind of makes it less cool in my opinion

i assume the reason for this change was to help make devs become more aware of the access modifiers? i don't know if my 'rant' is good enough to justify keeping pub

anyways asking the community (devs who code a lot of contracts on flow) might not hurt

[bjartek]

Has using no keyword to mean access(all) been considered?

[joshuahannan]

@bjartek I think that is a no-go because it public by default functions have been the cause of many hacks in solidity. it is good to require developers to make it explicit what the visibility is. I'm kind of ambivalent about pub or access(all) though, pub is nice and succinct, but I also like to idea of standardizing it to access() to fit better with the entitlement access syntax. Ultimately it doesn't really matter to me

[bjartek]

Long time developers on flow are frustrated because they feel the language is getting verbose and too cumbersome to write. Just see the above post. With the introduction of entitlements you need an correct entitlemed reference to do any hard regardless dont you? Can we please consider this and just not deny it in the name of security. The most secure program is no program at all.

[turbolent]

Thank you for your feedback everyone!

I guess this is a bit too late to raise any concerns around the change

Feedback is always welcome and encouraged, at any time!

anyways asking the community (devs who code a lot of contracts on flow) might not hurt

If possible, please consider following and voicing your opinion during times when proposals are looking for feedback and are voted on. The earlier the proposals receive feedback, the better. That does not make feedback after a FLIP has been approved any less important.

The FLIP was in active discussion for two months, and received no negative feedback. The team tried to encourage the community to join the discussion, e.g. through announcements on Discoerd, but did not receive any additional feedback.

Of course, the team understands that users might not have the time, capacity, or ability to join FLIP discussions. For example, even though I'm working full-time on this project, I personally cannot be involved in all FLIPs, and that is OK.

So, thank you for your feedback!

Often it is also difficult to see or estimate the impact that a proposal will have on real-world programs. FLIP authors and the team try their best to predict the impact, and in addition, provide preview builds to gather feedback from users.

Rest assured, the team is definitely not trying to / intentionally making the situation worse – it's quite the opposite: FLIP authors with their proposals, and the team in general, try to improve the language, based on its principles/values. Please check out https://cadence-lang.org, https://developers.flow.com/cadence/why, and https://developers.flow.com/cadence/intro#cadences-programming-language-pillars to learn more about what those are.

By nature, these goals are at odds! The "easier" a language is, and the more "freedom" it provides, the "unsafer" it is. The team has the difficult task of balancing the goals and making compromises. These tradeoffs are pointed out in the FLIPs, in the "User benefits" and "Drawbacks" sections. Cadence is in the "middle field" by choice, and the team completely agrees with "the most secure program is no program at all".

Decisions are not final. In particular for this FLIP and the context, reaching the Cadence 1.0 milestone is of the highest priority, but the milestone hasn't been reached yet. We still got time to further improve Cadence :-)

Discussing improvements to this FLIP on a closed PR for it is not the best place (e.g it is discoverable for the community). Let's move this discussion to the forum. It's a much better place to discuss and maybe come up with actionable work, like proposing a follow-up FLIP. I would love to discuss this change more, figure out what we can do about to make it work for everyone as best as possible, provide some explanations, and give some advice. For example, @bluesign opened a forum discussion to propose a follow-up improvement to another FLIP.

Finally, please provide feedback politely. If you can, be constructive. If you can't, e.g. you do not see any way to improve the situation, that's totally OK – please voice that concern. Just saying something is e.g. "horrible" and you "hate" it, or that a change "makes it less cool" is not useful, because it misses the why. Hearing about the effort for the migration, for example is useful, because it can be addressed.

➡️ Let's continue the discussion here: https://forum.onflow.org/t/follow-up-discussion-for-removing-pub-pub-set-and-priv-in-cadence-1-0/5235

[turbolent]

@bjartek Maybe I'm misunderstanding, but your feedback seems to be regarding entitlements, but this FLIP is about changing access modifiers.

please consider this and just not deny it in the name of security.

Could you please elaborate, ideally in a new forum post? For example, what do you mean with "this"? You're implying someone is denying something, but I don't quite follow.

[bjartek]

I was referring to @joshuahannan answer here #84 (comment).

I simply meant that if the concept of allowing no modifier at all to mean pub/access(all) have been consider. I just like terse expressions and typing access(all) all over the place is not very ergonomic friendly. I also added this command or something simliar to the forum.