diff --git a/neps/nep-0418.md b/neps/nep-0418.md new file mode 100644 index 000000000..d6788a155 --- /dev/null +++ b/neps/nep-0418.md @@ -0,0 +1,102 @@ +--- +NEP: 0418 +Title: Remove attached_deposit view panic +Author: Austin Abell +DiscussionsTo: https://github.com/nearprotocol/neps/pull/418 +Status: Approved +Type: Standards Track +Category: Tools +Version: 1.0.0 +Created: 18-Oct-2022 +Updated: 27-Jan-2023 +--- + +## Summary + +This proposal is to switch the behavior of the `attached_deposit` host function on the runtime from panicking in view contexts to returning 0. This results in a better devX because instead of having to configure an assertion that there was no attached deposit to a function call only for transactions and not view calls, which is impossible because you can send a transaction to any method, you could just apply this assertion without the runtime aborting in view contexts. + +## Motivation + +This will allow contract SDK frameworks to add the `attached_deposit == 0` assertion for every function on a contract by default. This behavior matches the Solidity/Eth payable modifier and will ensure that funds aren't sent accidentally to a contract in more cases than currently possible. + +This can't be done at a contract level because there is no way of checking if a function call is within view context to call `attached_deposit` conditionally. This means that there is no way of restricting the sending of funds to functions intended to be view only because the abort from within `attached_deposit` can't be caught and ignored from inside the contract. + +Initial discussion: https://near.zulipchat.com/#narrow/stream/295306-pagoda.2Fcontract-runtime/topic/attached_deposit.20view.20error + +## Rationale and alternatives + +The rationale for assigning `0u128` to the pointer (`u64`) passed into `attached_deposit` is that it's the least breaking change. + +The alternative of returning some special value, say `u128::MAX`, is that it would cause some unintended side effects for view calls using the `attached_deposit`. For example, if `attached_deposit` is called within a function, older versions of a contract that do not check the special value will return a result assuming that the attached deposit is `u128::MAX`. This is not a large concern since it would just be a view call, but it might be a bad UX in some edge cases, where returning 0 wouldn't be an issue. + +## Specification + +The error inside `attached_deposit` for view calls will be removed, and for all view calls, `0u128` will be set at the pointer passed in. + +## Reference Implementation + + +Currently, the implementation for `attached_deposit` is as follows: +```rust +pub fn attached_deposit(&mut self, balance_ptr: u64) -> Result<()> { + self.gas_counter.pay_base(base)?; + + if self.context.is_view() { + return Err(HostError::ProhibitedInView { + method_name: "attached_deposit".to_string(), + } + .into()); + } + self.memory_set_u128(balance_ptr, self.context.attached_deposit) +} +``` + +Which would just remove the check for `is_view` to no longer throw an error: + +```rust +pub fn attached_deposit(&mut self, balance_ptr: u64) -> Result<()> { + self.gas_counter.pay_base(base)?; + + self.memory_set_u128(balance_ptr, self.context.attached_deposit) +} +``` + +This assumes that in all cases, `self.context.attached_deposit` is set to 0 in all cases. This can be asserted, or just to be safe, can check if `self.context.is_view()` and set `0u128` explicitly. + +## Security Implications + +This won't have any implications outside of view calls, so this will not affect anything that is persisted on-chain. This only affects view calls. This can only have a negative side effect if a contract is under the assumption that `attached_deposit` will panic in view contexts. The possibility that this is done _and_ has some value connected with a view call result off-chain seems extremely unlikely. + +## Drawbacks + +This has a breaking change of the functionality of `attached_deposit` and affects the behavior of some function calls in view contexts if they use `attached_deposit` and no other prohibited host functions. + +## Future possibilities + +- The Rust SDK, as well as other SDKs, can add the `attached_deposit() == 0` check by default to all methods for safety of use. +- Potentially, other host functions can be allowed where reasonable values can be inferred. For example, `prepaid_gas`, `used_gas` could return 0. + +## Decision Context + +### 1.0.0 - Initial Version + +The initial version of NEP-418 was approved by Tools Working Group members on January 19, 2023 ([meeting recording](https://youtu.be/poVmblmc3L4)). + +#### Benefits + +- This will allow contract SDK frameworks to add the `attached_deposit == 0` assertion for every function on a contract by default. +- This behavior matches the Solidity/Eth payable modifier and will ensure that funds aren't sent accidentally to a contract in more cases than currently possible. +- Given that there is no way of checking if a function call is within view context to call `attached_deposit` conditionally, this NEP only changes a small surface of the API instead of introducing a new host function. + +#### Concerns + +| # | Concern | Resolution | Status | +| - | - | - | - | +| 1 | Proposal potentially triggers the protocol version change | It does not trigger the protocol version change. Current update could be considered a client-breaking change update. | Resolved | +| 2 | The contract can assume that `attached_deposit` will panic in view contexts. | The possibility that this is done _and_ has some value connected with a view call result off-chain seems extremely unlikely. | Won't Fix | +| 3 | Can we assume that in all view calls, the `attached_deposit` in the VMContext always zero? | Yes, there is no way to set `attached_deposit` in view calls context | Resolved | + +## Copyright +[copyright]: #copyright + +Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).