search

aeternity / aesophia

Stand alone compiler for the Sophia smart contract language
https://docs.aeternity.com/aesophia
ISC License
52 stars 19 forks source link

entrypoints without `stateful` annotation affecting state changes #343

Closed marc0olo closed 1 year ago

marc0olo commented 3 years ago

in the discussions today we got aware of the fact that currently entrypoints without the stateful annotation can still be used to:

this is documented accordingly here:

IMO we should definitely address this issue and do not allow the actions mentioned above in entrypoints without the stateful annotation

marc0olo commented 3 years ago

according to @hanssv this issue is better placed in the main aeternity repo. maybe somebody can move the issue there so that we don't need to close and re-open it.

UlfNorell commented 3 years ago

Depends on what guarantees you are after. The compiler could certainly disallow calling stateful entrypoints from non-stateful code, but since there are no statefulness checks in the VM, it would just be checking that you didn't call an endpoint that you claim is stateful.

radrow commented 3 years ago

This looks like a result of misunderstanding what "stateful" means. When an entrypoint is not stateful, it is guaranteed not to change the state of this particular contract. It doesn't apply to general stuff on the blockchain, including other contracts.

Since a remote contract can be arbitrary and statefulness is not a thing in the VM, putting these checks in the compiler would leave a hole that could be shamelessly exploited.

There are solutions to that:

If needed, pure keyword could be added that would be stricter than non-stateful by additionally forbidding state reads, chain queries and remote calls. But I see it an overkill.

Also, what's wrong with events?

marc0olo commented 3 years ago
  • Live with that and possibly fix the docs (the only thing you can do right atm)

I think the docs reflect it correctly. nothing to fix here. it seems like this wasn't discussed deep enough in the past and here we are now discussing it :-)

=> expectation (or assumption) of many devs is (or was) that non-stateful entrypoints cannot perform state changes (neither local, nor remote)

maybe we should first have a look again how the EVM and Solidity deal with that topic right now. according to @nikita-fuchs this isn't possible there.

Also, what's wrong with events?

not sure about that here. there was just a general assumption that it's not possible to emit events or perform any state change if an entrypoint isn't declared stateful

  • Add statefulness to the VM env (consensus-breaking)

I think we should start collecting a list of important consensus-breaking changes. because we already have a bunch of them discussed during the last few weeks.

at the moment I am not even sure if we already have an issue for the assembly operations we would like to have (cc @hanssv @bogdan-manole). this is probably sth. very important for us.

hanssv commented 3 years ago

@marc0olo Yes, #337 cover the bitwise VM-operations we've discussed.

radrow commented 3 years ago

I guess this is to be closed?

marc0olo commented 3 years ago

I guess this is to be closed?

if we aim to address this somewhere else I would prefer moving the issue to the respective repository over closing this. we shouldn't ignore/forget that topic :D

nikita-fuchs commented 3 years ago

Yep, please let's not lose track of this.

Marco Walz @.***> schrieb am Fr., 24. Sept. 2021, 08:54:

I guess this is to be closed?

if we aim to address this somewhere else I would prefer moving the issue to the respective repository over closing this. we shouldn't ignore/forget that topic :D

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aeternity/aesophia/issues/343#issuecomment-926390353, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZXRJH3SBRREIGT23EVELLUDQOBPANCNFSM5EL63X7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

radrow commented 1 year ago

Closing this as I don't see any reason to keep it. You have no control over remote contracts' statefulness (if ACI says it, it does not have to be true) and events do not change contract's state. If you would like to have an option to protect yourself from sending events, I would rather introduce a pure keyword and expand the restrictions to allow only stateless and pure operations.

marc0olo commented 1 year ago

to me it is still nonsense that it is allowed to execute stateful entrypoints from remote contracts in non-stateful entrypoints of the main contract.

doesn't sound like a Sophia topic as discussed above. but IMO it should be prevented somehow. I wouldn't expect any non-stateful entrypoint being able to change any state, also not on remote contracts.

nikita-fuchs commented 1 year ago

I'm absolutely with @marc0olo on this one !

hanssv commented 1 year ago

I had forgotten about this one...

First observation, if you are worried that a non-stateful entrypoint may possibly have side effects - then don't run it on-chain!

Then yes, depending on the mental picture of state, it could be a surprise that you are allowed to emit events and call stateful remote functions in a non-stateful context - but the documentation is clear on it so no urgency.

Options moving forward (in order of complexity/work):

marc0olo commented 1 year ago

I think it isn't an urgent topic and not worth the effort at this point of time. I am totally fine with "doing nothing" for now 👍