search

onflow / flips

Flow Improvement Proposals
26 stars 23 forks source link

FLIP for removing type requirements #118

Closed dsainati1 closed 1 year ago

dsainati1 commented 1 year ago

Adds a FLIP proposing to remove type requirements and change the semantics of event declarations in interfaces.

Part of https://github.com/onflow/cadence/issues/1283

austinkline commented 1 year ago

Would it be possible to use NFT/FT definitions as examples and show how they'd change based on this FLIP?

dsainati1 commented 1 year ago

cc @joshuahannan are the contract rewrites for the FT/NFT contracts in a state where we can use them for this FLIP?

joshuahannan commented 1 year ago

The new versions of the FT and NFT standards already don't use nested type requirements so you can take a look at those:

I'd like to merge those with the stable cadence feature branches so we can get them integrated into the preview release also, but just haven't been able to get started on it yet since I'm working on the core contracts right now

bluesign commented 1 year ago

I was thinking if I put events to an interface; I will control all events flowing from there.

As we turn into lets say single FT.Withdrawal event instead of FlowToken.Withdrawal; I can emit the event with (type: Type, address: Address, amount: UFix64) for example, people can listen a single event. ( very basic example )

So allowing others to emit would break the system.

dsainati1 commented 1 year ago

Practically speaking what @bluesign is suggesting would mean that when an event is declared inside an interface, it would only be available inside that interface's definition. E.g. if I had some event

contract interface I {
   event Foo()
}

Then Foo would only be available inside of I. Effectively it would be a private type definition. Would it be reasonable then to require it to be declared with access(self) to reflect this?

bluesign commented 1 year ago

Would it be reasonable then to require it to be declared with access(self) to reflect this?

I think emit already limits to defined contract scope when called. I think access(self) is not necessary in this case.

SupunS commented 1 year ago

That's an interesting point. I think previously the requirement of declaring events as pub/access(all) was to allow implementations to have access to it (especially in interfaces, for type requirements). But now that requirement is gone, maybe mandating events to be defined as non-public is a good move?

That way, there won't be a need to keep this special casing of emit statements (to not allow emitting out-of-scope events) as well. The usual access modifiers would handle it.