@bz-hashtag-0780 posted feedback to the FLIP which removed pub
, pub(set)
, and priv
here: https://github.com/onflow/flips/pull/84#issuecomment-1732627670
Let’s continue the discussion here!
@bz-hashtag-0780 posted feedback to the FLIP which removed pub
, pub(set)
, and priv
here: https://github.com/onflow/flips/pull/84#issuecomment-1732627670
Let’s continue the discussion here!
Since we want to move the discussion to here.
Is the discussion regarding making no modifier be the same as access(all)
dead?
My feelings on this (and to be clear these are my personal thoughts and not the position of the Cadence team as a whole) is that it is better to be explicit with regards to things like access modifiers. We talk a lot in breakout sessions about how “Cadence is read more often than it is written”, and as such we try to design language features and make language decisions based on how easy it is to read a program, and to reason about how it behaves. Allowing users to write types or fields with no access modifiers would make writing code faster, but it would obfuscate behavior by making it less apparent what access types and fields actually have, hence making the language harder to read and reason about.
The same principle was applied (just as another example of this, not trying to open a tangential discussion here) when we were considering adding an owner
entitlement that would give access to everything on a reference. While this would make writing code faster using entitlements, we would lose the property that a reference’s access is plainly visible in its type, and that users needed to list explicitly what entitlements a reference has, which is important for transparency in behavior.
Generally I tend to favor language changes that make reading code easier, even at the potential cost of ease of writing code, and this feels to me like a tradeoff that would add a small amount of utility at write-time at the cost of clarity at read-time.
Edit: if we are interested in saving time when writing, I’d suggest that we could add something to Cadence IDE tooling that might autofill access modifiers? I.e. if we detect an error complaining about a missing access modifier, we could have an autofix feature that automatically inserts an access(all)
modifier? This would have the best of both worlds, with clarity when reading but also speed when writing, perhaps.
Allowing users to write types or fields with no access modifiers would make writing code faster, but it would obfuscate behavior by making it less apparent what access types and fields actually have, hence making the language harder to read and reason about.
I agree with @sainati here, also lack of access modifier is used so frequently in other languages to imply something is private
Having said that, I think bringing back pub
as a shortcut to access(all)
later time is always on the table too I guess. ( after we merged to stable cadence, and avoid security pitfalls during upgrading ) It would not be a breaking change.
If we see that it is making the life of the developer living hell, we can always bring back pub
. But I think it will be adopted and we will not even see that anymore. ( As we are used to old cadence, I am still always typing pub
then changing, but I think it is just muscle memory )
Going over the feedback, these seem to be the themes:
Required time/effort to specify public access for new code
i have developed a deep frustration with the change because it just takes so much longer to write contracts in cadence
Required time/effort to migrate
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
Does it take longer because the access(all)
keyword is longer than pub
? Is your concern purely about the effort of writing, or also about reading? To address the writing concern, maybe tooling (IDE completion) could help? If reading is a concern: One of the goals of the FLIP was to unify the access modifier syntax to simply access(...)
. Maybe the simplification is not worth the effort it results in?
Could tooling help here? The type checker already reports a suggested fix for changing pub
to access(all)
(introduced in https://github.com/onflow/cadence/pull/2767).
Tooling could automate this further, by applying the migration automatically, but this is potentially dangerous: With the change to allow references to be downcasted, public functions and fields are not “protected” anymore by interfaces/restricted types.
You really want to audit your programs and double check if access was previously pub
, but actually protected, or really pub
in the sense that anyone can access it.
i assume the reason for this change was to help make devs become more aware of the access modifiers?
The intent was not to make developers aware of the access modifiers, but more so the changes to access control (safe downcasting/entitlements) requires developers to be aware and re-evaluate access modifiers.
Basically: See it as a chance to audit your code, and ensure that access control is and stays correct. If you have still have lot of access(all)
after the migration, it’s possible that there are more than there should be (I wouldn’t call it a “code smell”, but still something to look into).
Like the team has pointed out, e.g. in the last update, it very much understands that the effort for this migration is substantial. Still, we hope that this investment by the current community will be beneficial in the long-term and will help both existing developers writing new contracts, and new developers learning Cadence.
I don’t remember this was considered much. I don’t think it aligns with Cadence’s value of avoiding implicit behaviour and making things explicit.
Some developers might assume different behaviour, as the notion of default visibility actually differs in other languages. If they assume “private by default” behaviur, but there is an implicit “public by default” behaviour, this can lead to severe security issues.
Chiming in here
I think it should be required that an access modifier is used for the sake of readability.
There are two reasons for keeping pub:
access(all)
is very annoying to write. Although not as important as readability, I think we should keep this in mind as there are a lot of developers who do not like verbosity. There will not be as many contracts to read if people are frustrated by this and won’t writeaccess(all)
is harder to read anyway. When I scroll through a contract right now, if I see access()
my eyes immediately recognize this as “different” and it usually is contract or account (I’ve honestly almost never seen access(all)). When I see pub
, it’s easy on my eyes and easier to recognize immediately what’s going onWith that being said, I have encountered Cadence devs who do not like using pub
because it signifies more dangerous capabilities in other languages so they use access(all)
instead. But that’s why I think the choice is nice. This struggle is mostly a problem at the development level to have confidence you aren’t exposing bad things, whereas if you’re reading it and know pub
and access(all)
are the same, you are good to go
In the GitHub PR, @bjartek commented:
I just like terse expressions and typing
access(all)
all over the place is not very ergonomic friendly
That’s understandable, nobody likes additional work. However, typing an access modifier vs none at all, might be worth it here: Cadence is not a general purpose language, but specifically designed for smart contracts, where access control is the most important concern / security aspect. “Letting the guards down” to gain some ergonomics, while introducing potentially severe security issues, is not a good tradeoff unfortunately.
A related topic I wanted to bring up is @bz-hashtag-0780’s comment in the GitHub PR:
cadence is cool because it’s meant for rapid iteration, experimentation and being developer-friendly
That is definitely a design goal! We have several ideas on how Cadence can deliver more in this area, but have not gotten to it yet, because a stable Cadence has higher priority.
Once we reached the Cadence 1.0 milestone though, we can allocate more bandwidth to this effort again. Going “from prototype / idea to secure Mainnet contract” has been discussed since the very beginning of the Cadence design/development and is a big design goal.
I really hope we can look into gradual typing, e.g. like available for Python or e.g. TypeScript, to let developers focus on their core business logic (e.g. game loop) at first and where access control and types are optional, and let them incrementally transition to a mode where both are mandatory.
Maybe we can add something like access scopes ?
access(all) {
fun X(){
}
fun Y(){
}
}
This would not be a breaking change, can be added later too. Removes duplication of access modifiers, and groups functions and variables by access type.