entrypoints without stateful annotation affecting state changes #343

New Issue

2021-09-20T22:29:54+09:00

zxq9 commented

2021-09-20 22:29:54 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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

emit events
call stateful entrypoints of remote contracts

this is documented accordingly here:

https://github.com/aeternity/aesophia/blob/master/docs/sophia_features.md#stateful-functions

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

*Created by: marc0olo* in the discussions today we got aware of the fact that **_currently_** entrypoints without the `stateful` annotation can still be used to: - emit events - call `stateful` entrypoints of remote contracts this is documented accordingly here: - https://github.com/aeternity/aesophia/blob/master/docs/sophia_features.md#stateful-functions IMO we should definitely address this issue and do not allow the actions mentioned above in entrypoints without the `stateful` annotation

zxq9 commented

2021-09-20 22:35:07 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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.

*Created by: marc0olo* 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.

zxq9 commented

2021-09-20 22:38:03 +09:00

(Migrated from gitlab.com)

Created by: UlfNorell

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.

*Created by: UlfNorell* 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.

zxq9 commented

2021-09-21 23:04:12 +09:00

(Migrated from gitlab.com)

Created by: radrow

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:

Ban remote contracts from non-stateful contexts (inconvenient)
Add statefulness to the VM env (consensus-breaking)
Live with that and possibly fix the docs (the only thing you can do right atm)

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?

*Created by: radrow* 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: * Ban remote contracts from non-stateful contexts (inconvenient) * Add statefulness to the VM env (consensus-breaking) * Live with that and possibly fix the docs (the only thing you can do right atm) 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?

zxq9 commented

2021-09-21 23:28:24 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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.

*Created by: marc0olo* > * 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.

zxq9 commented

2021-09-22 00:16:52 +09:00

(Migrated from gitlab.com)

Created by: hanssv

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

*Created by: hanssv* @marc0olo Yes, #337 cover the bitwise VM-operations we've discussed.

zxq9 commented

2021-09-24 14:33:44 +09:00

(Migrated from gitlab.com)

Created by: radrow

I guess this is to be closed?

*Created by: radrow* I guess this is to be closed?

zxq9 commented

2021-09-24 15:54:04 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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

*Created by: marc0olo* > 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

zxq9 commented

2021-09-24 16:28:55 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

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://gitlab.com/gajumaristas/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.

*Created by: nikita-fuchs* 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://gitlab.com/gajumaristas/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>. > >

zxq9 commented

2023-08-05 20:22:31 +09:00

(Migrated from gitlab.com)

Created by: radrow

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.

*Created by: radrow* 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.

zxq9 commented

2023-08-06 01:15:43 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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.

*Created by: marc0olo* 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.

zxq9 commented

2023-08-15 21:27:09 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

I'm absolutely with @marc0olo on this one !

*Created by: nikita-fuchs* I'm absolutely with @marc0olo on this one !

zxq9 commented

2023-08-15 22:56:52 +09:00

(Migrated from gitlab.com)

Created by: hanssv

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):

do nothing (my favourite action!)
change the compiler to be stricter, but still don't enforce this strictly on-chain (sounds reasonable, again don't run non-stateful entrypoints on chain and you'll be 100% safe).
change the compiler and the byte code/fate code to track statefulness - and then enforce the "don't run non-stateful entrypoints on-chain" in FATE (in this case we wouldn't have to track what the byte code actually does which saves a lot of work)
change the compiler and the byte code/fate code to track statefulness - and then change FATE to check that indeed only stateful code does stateful things! (lots of work riddled all through the FATE VM so a bit error prone)

*Created by: hanssv* 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): * do nothing (my favourite action!) * change the compiler to be stricter, but still don't enforce this strictly on-chain (sounds reasonable, again don't run non-stateful entrypoints on chain and you'll be 100% safe). * change the compiler and the byte code/fate code to track statefulness - and then enforce the "don't run non-stateful entrypoints on-chain" in FATE (in this case we wouldn't have to track what the byte code actually does which saves a lot of work) * change the compiler and the byte code/fate code to track statefulness - and then change FATE to check that indeed only stateful code does stateful things! (lots of work riddled all through the FATE VM so a bit error prone)

zxq9 commented

2023-08-16 04:23:30 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

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 👍

*Created by: marc0olo* 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 👍

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QPQ-AG/sophia#343

entrypoints without stateful annotation affecting state changes #343

entrypoints without `stateful` annotation affecting state changes #343