Use seal_transfer() for .transfer() and .send()

After some soul-searching, it might be best if the transfer() and send()
functions use seal_transfer.

1. Using seal_call() has some gas cost overhead
2. Using seal_call() requires the receiving contract to have a
   receive() external payable { /* .. */ } thing.

Signed-off-by: Sean Young <sean@mess.org>

docs/language.rst

@@ -2695,8 +2695,10 @@ Here is an example:
     }
 
 .. note::
-    This uses the ``seal_call()`` mechanism rather than ``seal_transfer()``, since
-    Solidity expects the ``receive()`` function to be called on receipt.
+    On Subtrate, this uses the ``seal_transfer()`` mechanism rather than ``seal_call()``, since this
+    does not come with gas overhead. This means the ``receive()`` function is not required in the
+    receiving contract, and it will not be called if it is present. If you want the ``receive()``
+    function to be called, use ``address.call{value: 100}("")`` instead.
 
 
 Builtin Functions and Variables

src/codegen/cfg.rs

@@ -103,6 +103,12 @@ pub enum Instr {
         gas: Expression,
         callty: CallTy,
     },
+    /// Value transfer; either <address>.send() or <address>.transfer()
+    ValueTransfer {
+        success: Option<usize>,
+        address: Expression,
+        value: Expression,
+    },
     /// ABI decoder encoded data. If decoding fails, either jump to exception
     /// or abort if this is None.
     AbiDecode {
@@ -808,6 +814,21 @@ impl ControlFlowGraph {
                     )
                 }
             }
+            Instr::ValueTransfer {
+                success,
+                address,
+                value,
+            } => {
+                format!(
+                    "{} = value transfer address:{} value:{}",
+                    match success {
+                        Some(i) => format!("%{}", self.vars[i].id.name),
+                        None => "_".to_string(),
+                    },
+                    self.expr_to_string(contract, ns, address),
+                    self.expr_to_string(contract, ns, value),
+                )
+            }
             Instr::AbiDecode {
                 res,
                 tys,

src/codegen/expression.rs

@@ -780,18 +780,29 @@ pub fn expression(
                 &Type::Bool,
             );
 
-            cfg.add(
-                vartab,
-                Instr::ExternalCall {
-                    success: Some(success),
-                    address: Some(address),
-                    payload: Expression::BytesLiteral(*loc, Type::DynamicBytes, vec![]),
-                    args: Vec::new(),
-                    value,
-                    gas: Expression::NumberLiteral(*loc, Type::Uint(64), BigInt::zero()),
-                    callty: CallTy::Regular,
-                },
-            );
+            if ns.target == Target::Substrate {
+                cfg.add(
+                    vartab,
+                    Instr::ValueTransfer {
+                        success: Some(success),
+                        address,
+                        value,
+                    },
+                );
+            } else {
+                cfg.add(
+                    vartab,
+                    Instr::ExternalCall {
+                        success: Some(success),
+                        address: Some(address),
+                        payload: Expression::BytesLiteral(*loc, Type::DynamicBytes, vec![]),
+                        args: Vec::new(),
+                        value,
+                        gas: Expression::NumberLiteral(*loc, Type::Uint(64), BigInt::zero()),
+                        callty: CallTy::Regular,
+                    },
+                );
+            }
 
             Expression::Variable(*loc, Type::Bool, success)
         }
@@ -799,18 +810,29 @@ pub fn expression(
             let address = expression(&args[0], cfg, contract_no, ns, vartab);
             let value = expression(&args[1], cfg, contract_no, ns, vartab);
 
-            cfg.add(
-                vartab,
-                Instr::ExternalCall {
-                    success: None,
-                    address: Some(address),
-                    payload: Expression::BytesLiteral(*loc, Type::DynamicBytes, vec![]),
-                    args: Vec::new(),
-                    value,
-                    gas: Expression::NumberLiteral(*loc, Type::Uint(64), BigInt::zero()),
-                    callty: CallTy::Regular,
-                },
-            );
+            if ns.target == Target::Substrate {
+                cfg.add(
+                    vartab,
+                    Instr::ValueTransfer {
+                        success: None,
+                        address,
+                        value,
+                    },
+                );
+            } else {
+                cfg.add(
+                    vartab,
+                    Instr::ExternalCall {
+                        success: None,
+                        address: Some(address),
+                        payload: Expression::BytesLiteral(*loc, Type::DynamicBytes, vec![]),
+                        args: Vec::new(),
+                        value,
+                        gas: Expression::NumberLiteral(*loc, Type::Uint(64), BigInt::zero()),
+                        callty: CallTy::Regular,
+                    },
+                );
+            }
 
             Expression::Poison
         }

src/codegen/reaching_definitions.rs

@@ -136,6 +136,9 @@ fn instr_transfers(block_no: usize, block: &BasicBlock) -> Vec<Vec<Transfer>> {
             }
             | Instr::Constructor {
                 success: None, res, ..
+            }
+            | Instr::ValueTransfer {
+                success: Some(res), ..
             } => set_var(&[*res]),
             Instr::ClearStorage { storage: dest, .. }
             | Instr::SetStorageBytes { storage: dest, .. }

src/codegen/vector_to_slice.rs

@@ -97,7 +97,8 @@ fn find_writable_vectors(
             | Instr::Unreachable
             | Instr::Print { .. }
             | Instr::AbiEncodeVector { .. }
-            | Instr::AssertFailure { .. } => {
+            | Instr::AssertFailure { .. }
+            | Instr::ValueTransfer { .. } => {
                 apply_transfers(&block.transfers[instr_no], vars, writable);
             }
         }

src/emit/mod.rs

@@ -237,6 +237,18 @@ pub trait TargetRuntime<'a> {
         ty: ast::CallTy,
     );
 
+    /// send value to address
+    fn value_transfer<'b>(
+        &self,
+        _contract: &Contract<'b>,
+        _function: FunctionValue,
+        _success: Option<&mut BasicValueEnum<'b>>,
+        _address: PointerValue<'b>,
+        _value: IntValue<'b>,
+    ) {
+        unimplemented!();
+    }
+
     /// builtin expressions
     fn builtin<'b>(
         &self,
@@ -3972,6 +3984,42 @@ pub trait TargetRuntime<'a> {
                             callty.clone(),
                         );
                     }
+                    Instr::ValueTransfer {
+                        success,
+                        address,
+                        value,
+                    } => {
+                        let value = self
+                            .expression(contract, value, &w.vars, function)
+                            .into_int_value();
+                        let address = self
+                            .expression(contract, address, &w.vars, function)
+                            .into_int_value();
+
+                        let addr = contract.builder.build_array_alloca(
+                            contract.context.i8_type(),
+                            contract
+                                .context
+                                .i32_type()
+                                .const_int(contract.ns.address_length as u64, false),
+                            "address",
+                        );
+
+                        contract.builder.build_store(
+                            contract.builder.build_pointer_cast(
+                                addr,
+                                address.get_type().ptr_type(AddressSpace::Generic),
+                                "address",
+                            ),
+                            address,
+                        );
+                        let success = match success {
+                            Some(n) => Some(&mut w.vars.get_mut(n).unwrap().value),
+                            None => None,
+                        };
+
+                        self.value_transfer(contract, function, success, addr, value);
+                    }
                     Instr::AbiEncodeVector {
                         res,
                         tys,

src/emit/substrate.rs

@@ -97,6 +97,7 @@ impl SubstrateTarget {
             "seal_tombstone_deposit",
             "seal_terminate",
             "seal_deposit_event",
+            "seal_transfer",
         ]);
 
         c
@@ -366,6 +367,18 @@ impl SubstrateTarget {
             Some(Linkage::External),
         );
 
+        contract.module.add_function(
+            "seal_transfer",
+            contract.context.i32_type().fn_type(
+                &[
+                    u8_ptr, u32_val, // address ptr and len
+                    u8_ptr, u32_val, // value ptr and len
+                ],
+                false,
+            ),
+            Some(Linkage::External),
+        );
+
         contract.module.add_function(
             "seal_value_transferred",
             contract
@@ -3278,6 +3291,103 @@ impl<'a> TargetRuntime<'a> for SubstrateTarget {
         }
     }
 
+    /// Send value to address
+    fn value_transfer<'b>(
+        &self,
+        contract: &Contract<'b>,
+        function: FunctionValue,
+        success: Option<&mut BasicValueEnum<'b>>,
+        address: PointerValue<'b>,
+        value: IntValue<'b>,
+    ) {
+        // balance is a u128
+        let value_ptr = contract
+            .builder
+            .build_alloca(contract.value_type(), "balance");
+        contract.builder.build_store(value_ptr, value);
+
+        let scratch_buf = contract.builder.build_pointer_cast(
+            contract.scratch.unwrap().as_pointer_value(),
+            contract.context.i8_type().ptr_type(AddressSpace::Generic),
+            "scratch_buf",
+        );
+        let scratch_len = contract.scratch_len.unwrap().as_pointer_value();
+
+        contract.builder.build_store(
+            scratch_len,
+            contract
+                .context
+                .i32_type()
+                .const_int(SCRATCH_SIZE as u64, false),
+        );
+
+        // do the actual call
+        let ret = contract
+            .builder
+            .build_call(
+                contract.module.get_function("seal_transfer").unwrap(),
+                &[
+                    address.into(),
+                    contract
+                        .context
+                        .i32_type()
+                        .const_int(contract.ns.address_length as u64, false)
+                        .into(),
+                    contract
+                        .builder
+                        .build_pointer_cast(
+                            value_ptr,
+                            contract.context.i8_type().ptr_type(AddressSpace::Generic),
+                            "value_transfer",
+                        )
+                        .into(),
+                    contract
+                        .context
+                        .i32_type()
+                        .const_int(contract.ns.value_length as u64, false)
+                        .into(),
+                ],
+                "",
+            )
+            .try_as_basic_value()
+            .left()
+            .unwrap()
+            .into_int_value();
+
+        let is_success = contract.builder.build_int_compare(
+            IntPredicate::EQ,
+            ret,
+            contract.context.i32_type().const_zero(),
+            "success",
+        );
+
+        if let Some(success) = success {
+            // we're in a try statement. This means:
+            // do not abort execution; return success or not in success variable
+            *success = is_success.into();
+        } else {
+            let success_block = contract.context.append_basic_block(function, "success");
+            let bail_block = contract.context.append_basic_block(function, "bail");
+
+            contract
+                .builder
+                .build_conditional_branch(is_success, success_block, bail_block);
+
+            contract.builder.position_at_end(bail_block);
+
+            self.assert_failure(
+                contract,
+                scratch_buf,
+                contract
+                    .builder
+                    .build_load(scratch_len, "string_len")
+                    .into_int_value(),
+            );
+
+            contract.builder.position_at_end(success_block);
+        }
+    }
+
     fn return_data<'b>(&self, contract: &Contract<'b>) -> PointerValue<'b> {
         let scratch_buf = contract.builder.build_pointer_cast(
             contract.scratch.unwrap().as_pointer_value(),

tests/substrate.rs

@@ -80,6 +80,7 @@ enum SubstrateExternal {
     seal_caller,
     seal_tombstone_deposit,
     seal_deposit_event,
+    seal_transfer,
 }
 
 pub struct Event {
@@ -520,6 +521,45 @@ impl Externals for TestRuntime {
 
                 Ok(ret)
             }
+            Some(SubstrateExternal::seal_transfer) => {
+                let address_ptr: u32 = args.nth_checked(0)?;
+                let address_len: u32 = args.nth_checked(1)?;
+                let value_ptr: u32 = args.nth_checked(2)?;
+                let value_len: u32 = args.nth_checked(3)?;
+
+                let mut address = [0u8; 32];
+
+                if address_len != 32 {
+                    panic!("seal_transfer: len = {}", address_len);
+                }
+
+                if let Err(e) = self.vm.memory.get_into(address_ptr, &mut address) {
+                    panic!("seal_transfer: {}", e);
+                }
+
+                let mut value = [0u8; 16];
+
+                if value_len != 16 {
+                    panic!("seal_transfer: len = {}", value_len);
+                }
+
+                if let Err(e) = self.vm.memory.get_into(value_ptr, &mut value) {
+                    panic!("seal_transfer: {}", e);
+                }
+
+                let value = u128::from_le_bytes(value);
+
+                if !self.accounts.contains_key(&address) {
+                    // substrate would return TransferFailed
+                    return Ok(Some(RuntimeValue::I32(0x5)));
+                }
+
+                if let Some(acc) = self.accounts.get_mut(&address) {
+                    acc.1 += value;
+                }
+
+                Ok(Some(RuntimeValue::I32(0)))
+            }
             Some(SubstrateExternal::seal_instantiate) => {
                 let codehash_ptr: u32 = args.nth_checked(0)?;
                 let codehash_len: u32 = args.nth_checked(1)?;
@@ -856,6 +896,7 @@ impl ModuleImportResolver for TestRuntime {
             "seal_caller" => SubstrateExternal::seal_caller,
             "seal_tombstone_deposit" => SubstrateExternal::seal_tombstone_deposit,
             "seal_deposit_event" => SubstrateExternal::seal_deposit_event,
+            "seal_transfer" => SubstrateExternal::seal_transfer,
             _ => {
                 panic!("{} not implemented", field_name);
             }

tests/substrate_tests/value.rs

@@ -840,7 +840,8 @@ fn send_and_transfer() {
 
     runtime.function("step1", Vec::new());
 
-    assert_eq!(runtime.vm.output, false.encode());
+    // no receive() required for send/transfer
+    assert_eq!(runtime.vm.output, true.encode());
 
     for (address, account) in runtime.accounts {
         if address == runtime.vm.address {
@@ -906,7 +907,7 @@ fn send_and_transfer() {
 
     runtime.constructor(0, Vec::new());
 
-    runtime.function_expect_return("step1", Vec::new(), 1);
+    runtime.function("step1", Vec::new());
 
     for (address, account) in runtime.accounts {
         if address == runtime.vm.address {