Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject unexpected transfers of value (was: Add an equivalent of "payable" to the runtime) #836

Open
Stebalien opened this issue Nov 11, 2022 · 7 comments
Assignees

Comments

@Stebalien
Copy link
Member

Specifically:

rt.payable() where a call gets reverted if:

  1. The balance sent to the actor was non-zero.
  2. rt.payable was not called at some point.

We could, alternatively, have a rt.not_payable(). That's the "less breaking" approach.

@vyzo
Copy link
Contributor

vyzo commented Nov 11, 2022

The negated approach is saner, this is special functionality and should thus be optin.

@anorth
Copy link
Member

anorth commented Nov 13, 2022

I think there's reasonable justification for doing the breaking approach, if we can do it soon. Receiving funds unexpectedly would very likely lead to their loss. I would say that being able to receive funds is the special functionality.

@vyzo
Copy link
Contributor

vyzo commented Nov 13, 2022

Thats a reasonable argument, but is there precedent for it?

More importantly for FEVM, isnt accepting funds the default mode?
Code often doesnt even get to run there with the gas limit convention, so a contract wouldnt have the chance to accept funds by default.

@anorth
Copy link
Member

anorth commented Nov 14, 2022

Just to confirm, @Stebalien, given the location of this issue, you're proposing this as a safety feature for the built-in actors runtime (which is essentially just a support library). It's not something that would be available or enforced for any user actors, though of course they could copy the pattern.

@anorth
Copy link
Member

anorth commented Nov 14, 2022

Related discussion about being able to refuse bare sends of value (which would require VM support): filecoin-project/ref-fvm#835

@Stebalien
Copy link
Member Author

Just to confirm, @Stebalien, given the location of this issue, you're proposing this as a safety feature for the built-in actors runtime (which is essentially just a support library). It's not something that would be available or enforced for any user actors, though of course they could copy the pattern.

Yes.

More importantly for FEVM, isnt accepting funds the default mode?

Yes. However, the default in solidity is to reject funds.

@anorth anorth changed the title Add an equivalent of "payable" to the runtime Reject unexpected transfers of value (was: Add an equivalent of "payable" to the runtime) Mar 2, 2023
@anorth
Copy link
Member

anorth commented Mar 2, 2023

I've renamed this "Reject unexpected transfers of value" to reflect the behavioural change. I concur that support in the Runtime would be a good way to get it. This will become valuable as user-programmed contracts/actors emerge, and we can do the development/user community a favour by preventing the built-in actors from accepting their funds.

@anorth anorth added the P2 label Mar 2, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Network nv19 Mar 2, 2023
@anorth anorth moved this from 📋 Backlog to 🏗 In progress in Network nv19 Mar 9, 2023
@anorth anorth added the blocked label Apr 3, 2023
@anorth anorth moved this from 🏗 In progress to 📋 Backlog in Network nv19 Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

4 participants