Open joshuahannan opened 5 months ago
This causes a bug in the new token standards that I missed testing while we were iterating. Just fell through the cracks, so I take responsibility for missing it. As of now, this either would require an update to Cadence to allow or we'll have to update the token standard with a workaround, but I'm curious to hear if this behavior by the language is intentional or not
@SupunS @turbolent Can you take a look at this? This is preventing people from being able to update their contracts with all the functionality in the token standards
In my view, it is correct to say that any function with access(contract)
can only be accessed by the contract interface itself only. I guess what we are looking for here is an access modifier in between "contract" and "public" that can be seen by its implementations (i.e: something similar to protected
in Java).
But then again, default functions defined with access(contract)
are "partially visible" to their implementations already (related to https://github.com/onflow/cadence/issues/3207). I think we need to clean this up.
See Discord discussion: https://discord.com/channels/613813861610684416/1222979306880372777
This is different than the discussion about the Burner
contract IMO. This is a function that, if there was an override implementation, would be accessible in the implementing contract, so it doesn't really make sense to not allow the default implementation to be accessed in the implementing contract. I would think that this should be allowed by Cadence, whereas I agree that the case of the interface contract being able to call access(contract)
functions in implementations is okay.
Like, if I were to make this function access(all)
, then my implementation contract could call the function default implementation via my contract (the implementation) even if my contract hasn't provided an implementation for the function. I don't see why an access(contract)
function should be any different.
I wouldn't consider the current behaviour a bug: A call of a access(contract)
function (it does not matter if it happens to declare a default implementation for concrete types) from outside of the contract fails as expected.
I guess we could extend the access control mechanism and allow calls of the default function also from any code of a contract that defines a concrete type that implements the interface with the default function.
Is this really needed though? It seems to complicate both the language semantics and implementation even further.
As already pointed out in the Discord thread, the usage of access(contract)
in an interface and its existing behaviour (Burner example) is already surprising. Adding even more functionality might not be a good idea.
I was thinking we should even reject interface default functions with access(contract)
, because right now its a bit inconsistent/confusing. i.e: they would have to be public
I was thinking we should even reject interface default functions with
access(contract)
, because right now its a bit inconsistent/confusing. i.e: they would have to be public
Yeah, I made a similar suggestion in the Discord thread, we might want to restrict this further instead of extending it, potentially even forbidding access(contract)
(maybe also access(self)
?) in interfaces, to avoid confusion.
Well, I definitely wouldn't want to restrict it any more because that would break the Burner
contract! 😅 If we were to restrict it more, we would need a new access modifier that can allow the behavior that we are asking for here and with the Burner
contract.
For this issue specifically, the problem is with the NFT standard needing to be able to emit a standard event, but we want that standard event to only be able to be triggered by a contract for its own NFTs. I've just made the function access(all)
for now to make the event emission possible. That is acceptable, but isn't ideal because now that means that anyone with an authorized reference to any NFT can call it and emit junk events.
interface can call the implementation but implementation cannot call the interface? I think both should be able to call each other ( without being public )
my mental model was when interface with default conditions is; there is no interface. Implementation and interface becomes one. Depending on how you access the resource/contract ( via interface or via type ), you see a limited view of the merged state.
Current Behavior
A contract interface defines an
access(contract)
function and gives it a default implementation. A contract that implements the contract interface does not provide an implementation for the function, intending to use the default implementation, but when code in the implementing contract tries to call the function, there is an error:Expected Behavior
The function should be able to be called by the implementation since it is
access(contract)
Steps To Reproduce
Deploy this contract interface:
pub contract CallFunction: ContractFunction { init() { CallFunction.callableFromImpl() } }