Constructors on Polkadot can be non-payable (#1460)

We use the `seal1` `instantiate` runtime API, and with this runtime host
function there is no need to send the minimum balance on instantiation
(the comment in the docs is not correct).

Hence, constructors on Polkadot can and should actually be non-payble

---------

Signed-off-by: xermicus <cyrill@parity.io>

docs/language/functions.rst

@@ -162,12 +162,8 @@ A constructor can be marked ``payable``, in which case value can be passed with
 constructor.
 
 .. note::
-    If value is sent to a non-payable function on Polkadot, the call will be
-    reverted. However there is no refund performed, so value will remain with the callee.
+    If value is sent to a non-payable function on Polkadot, the call will be reverted.
 
-    ``payable`` on constructors is not enforced on Polkadot,. Funds are needed
-    for storage rent and there is a minimum deposit needed for the contract. As a result,
-    constructors always receive value on Polkadot.
 
 Overriding function selector
 ____________________________

integration/polkadot/UniswapV2Factory.sol

@@ -12,7 +12,7 @@ contract UniswapV2Factory is IUniswapV2Factory {
 
     event PairCreated(address token0, address token1, address pair, uint);
 
-    constructor(address _feeToSetter) public {
+    constructor(address _feeToSetter) public payable {
         feeToSetter = _feeToSetter;
     }
 

integration/polkadot/UniswapV2Pair.sol

@@ -58,7 +58,7 @@ contract UniswapV2Pair is IUniswapV2Pair, UniswapV2ERC20 {
     );
     event Sync(uint112 reserve0, uint112 reserve1);
 
-    constructor() public {
+    constructor() public payable {
         factory = msg.sender;
     }
 

integration/polkadot/balances.sol

@@ -1,4 +1,6 @@
 contract balances {
+    constructor() payable {}
+
     function get_balance() public view returns (uint128) {
         return address(this).balance;
     }

integration/polkadot/create_contract.sol

@@ -1,4 +1,6 @@
 contract creator {
+    constructor() payable {}
+
     child_create_contract public c;
 
     function create_child() public {

integration/polkadot/runtime_errors.sol

@@ -63,9 +63,9 @@ contract RuntimeErrors {
         e.callee_func();
     }
 
-    // contract creation failed (contract was deplyed with no value)
+    // contract creation failed (nonpayable constructor received value)
     function create_child() public {
-        c = new child_runtime_errors();
+        c = new child_runtime_errors{value: 1}();
     }
 
     // non payable function dont_pay_me received value
@@ -126,7 +126,7 @@ contract callee_error {
 }
 
 contract child_runtime_errors {
-    constructor() payable {}
+    constructor() {}
 
     function say_my_name() public pure returns (string memory) {
         return "child_runtime_errors";

integration/polkadot/runtime_errors.spec.ts

@@ -32,7 +32,7 @@ describe('Deploy runtime_errors.sol and test the debug buffer', () => {
         expect(res3).toContain(`runtime_error: external call failed in runtime_errors.sol:63:9-24`)
 
         let res4 = await debug_buffer(conn, contract, `create_child`);
-        expect(res4).toContain(`runtime_error: contract creation failed in runtime_errors.sol:68:13-39`)
+        expect(res4).toContain(`runtime_error: contract creation failed in runtime_errors.sol:68`)
 
         let res5 = await debug_buffer(conn, contract, `set_storage_bytes`, [])
         expect(res5).toContain(`runtime_error: storage index out of bounds in runtime_errors.sol:39:11-12`)

integration/subxt-tests/src/cases/flipper.rs

@@ -19,7 +19,7 @@ async fn case() -> anyhow::Result<()> {
         .deploy(
             &api,
             sp_keyring::AccountKeyring::Alice,
-            10_u128.pow(16),
+            0_u128,
             &|t: &ContractMessageTranscoder| t.encode::<_, String>("new", ["true".into()]).unwrap(),
         )
         .await?;

src/codegen/cfg.rs

@@ -1638,11 +1638,7 @@ fn function_cfg(
 
     cfg.public = ns.function_externally_callable(contract_no, function_no);
     cfg.ty = func.ty;
-    cfg.nonpayable = if ns.target.is_polkadot() {
-        !func.is_constructor() && !func.is_payable()
-    } else {
-        !func.is_payable()
-    };
+    cfg.nonpayable = !func.is_payable();
 
     // populate the argument variables
     populate_arguments(func, &mut cfg, &mut vartab);

src/codegen/dispatch/polkadot.rs

@@ -375,10 +375,8 @@ impl<'a> Dispatch<'a> {
     }
 
     /// Insert a trap into the cfg, if the function `func_no` is not payable but received value anyways.
-    /// Constructors always receive endowment.
     fn abort_if_value_transfer(&mut self, func_no: usize) {
-        if !self.all_cfg[func_no].nonpayable || self.all_cfg[func_no].ty == FunctionTy::Constructor
-        {
+        if !self.all_cfg[func_no].nonpayable {
             return;
         }
 

src/emit/polkadot/target.rs

@@ -853,22 +853,10 @@ impl<'a> TargetRuntime<'a> for PolkadotTarget {
             .builder
             .build_alloca(binary.value_type(ns), "balance");
 
-        // balance is a u128, make sure it's enough to cover existential_deposit
-        if let Some(value) = contract_args.value {
-            binary.builder.build_store(value_ptr, value);
-        } else {
-            let scratch_len = binary.scratch_len.unwrap().as_pointer_value();
-
-            binary
-                .builder
-                .build_store(scratch_len, i32_const!(ns.value_length as u64));
-
-            call!(
-                "minimum_balance",
-                &[value_ptr.into(), scratch_len.into()],
-                "minimum_balance"
-            );
-        }
+        let value = contract_args
+            .value
+            .unwrap_or_else(|| binary.value_type(ns).const_zero());
+        binary.builder.build_store(value_ptr, value);
 
         // code hash
         let codehash = binary.emit_global_string(

tests/codegen_testcases/solidity/polkadot_dispatch.sol

@@ -262,8 +262,8 @@ contract overloaded {
 	// CHECK: 	 = call overloaded::overloaded::receive 
 	// CHECK: 	return data (alloc bytes len uint32 0), data length: uint32 0
 
-	constructor foo() {}
-	constructor bar() {}
+	constructor foo() payable {}
+	constructor bar() payable {}
 	function f() public payable {}
 	function f(uint256 i) public pure {}
 	fallback() external {}
@@ -321,3 +321,24 @@ contract simple {
 
 	function foo() public pure {}
 }
+
+contract nonpayableConstructor {
+	// BEGIN-CHECK: Contract: nonpayableConstructor
+	
+	// CHECK: # function polkadot_deploy_dispatch public:false selector: nonpayable:false
+
+	// CHECK: switch %selector.temp.54:
+    // CHECK: case uint32 2371928013: goto block #3
+
+	// CHECK: block3: # func_0_dispatch
+	// CHCEK: branchcond (unsigned more %value.temp.52 > uint128 0), block4, block5
+
+	// CHECK: block4: # func_0_got_value
+ 	// CHECK: print 
+ 	// CHECK: assert-failure
+	// CHECK: block5: # func_0_no_value
+ 	// CHECK: = call nonpayableConstructor::nonpayableConstructor::constructor::cdbf608d 
+	
+	constructor () {}
+	function foo() public pure {}
+}

tests/polkadot.rs

@@ -893,7 +893,6 @@ impl MockSubstrate {
     ///
     /// `input` must contain the selector fo the constructor.
     pub fn raw_constructor(&mut self, input: Vec<u8>) {
-        self.0.data_mut().transferred_value = 20000;
         self.invoke("deploy", input).unwrap();
     }
 
@@ -1018,9 +1017,8 @@ impl MockSubstrate {
 
 /// Build all contracts foud in `src` and set up a mock runtime.
 ///
-/// The mock runtime will contain a contract account for each contract in `src`:
-/// * Each account will have a balance of 20'000
-/// * However, constructors are _not_ called, therefor the storage will not be initialized
+/// The mock runtime will contain a contract account for each contract in `src`.
+/// Constructors are _not_ called, therefore the storage will not be initialized.
 pub fn build_solidity(src: &str) -> MockSubstrate {
     build_solidity_with_options(src, false, true)
 }

tests/polkadot_tests/value.rs

@@ -11,6 +11,7 @@ fn external_call_value() {
         contract b {
             a f;
 
+            constructor() payable {}
             function step1() public {
                 f = new a();
             }
@@ -26,17 +27,20 @@ fn external_call_value() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
     runtime.function("step2", Vec::new());
 
-    // Minimum balance + transferred value = 500 + 1023
-    assert_eq!(runtime.balance(2), 1523);
+    // Transferred value = 1023
+    assert_eq!(runtime.balance(2), 1023);
 
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
                 a f = new a();
                 try f.test{value: 1023}(501) {
@@ -54,12 +58,14 @@ fn external_call_value() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
 
-    // Minimum balance + transferred value = 500 + 1023
-    assert_eq!(runtime.balance(2), 1523);
+    // Transferred value = 1023
+    assert_eq!(runtime.balance(2), 1023);
 }
 
 #[test]
@@ -67,26 +73,30 @@ fn constructor_value() {
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
-                a f = new a();
+                a f = new a{value: 500}();
             }
         }
 
         contract a {
+            constructor() payable {}
             function test(int32 l) public payable {
             }
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-
     assert_eq!(runtime.balance(2), 500);
 
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
                 a f = (new a){value: 0}();
             }
@@ -98,35 +108,40 @@ fn constructor_value() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-
     assert_eq!(runtime.balance(2), 0);
 
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
                 a f = new a{value: 499}();
             }
         }
 
         contract a {
+            constructor() payable {}
             function test(int32 l) public payable {
             }
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-
     assert_eq!(runtime.balance(2), 499);
 
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
                 try (new a{value: 511})() {
                     //
@@ -138,20 +153,23 @@ fn constructor_value() {
         }
 
         contract a {
+            constructor() payable {}
             function test(int32 l) public payable {
             }
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-
     assert_eq!(runtime.balance(2), 511);
 
     let mut runtime = build_solidity(
         r##"
         contract b {
+            constructor() payable {}
             function step1() public {
                 try (new a){value: 511}() returns (a) {
                     //
@@ -163,15 +181,17 @@ fn constructor_value() {
         }
 
         contract a {
+            constructor() payable {}
             function test(int32 l) public payable {
             }
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-
     assert_eq!(runtime.balance(2), 511)
 }
 
@@ -312,7 +332,7 @@ fn balance() {
         contract b {
             other o;
 
-            constructor() public {
+            constructor() public payable {
                 o = new other();
             }
 
@@ -332,14 +352,16 @@ fn balance() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
+
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-    // Constructor received 20000 by default, however 500 were sent to "o"
-    assert_eq!(runtime.output(), 19500u128.to_le_bytes());
+    // Constructor received 20000, 0 were sent to "o"
+    assert_eq!(runtime.output(), 20000u128.to_le_bytes());
 
     runtime.function("step2", Vec::new());
-
-    assert_eq!(runtime.output(), 500u128.to_le_bytes());
+    assert_eq!(runtime.output(), 0u128.to_le_bytes());
 }
 
 #[test]
@@ -348,6 +370,7 @@ fn selfdestruct() {
         r##"
         contract c {
             other o;
+            constructor() payable {}
             function step1() public {
                 o = new other{value: 511}();
             }
@@ -358,13 +381,17 @@ fn selfdestruct() {
         }
 
         contract other {
+            constructor() payable {}
             function goaway(address payable recipient) public returns (bool) {
                 selfdestruct(recipient);
             }
         }"##,
     );
+
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
     assert_eq!(runtime.balance(0), 20000 - 511);
 
@@ -379,7 +406,7 @@ fn send_and_transfer() {
         contract c {
             other o;
 
-            constructor() public {
+            constructor() public payable {
                 o = new other();
             }
 
@@ -394,20 +421,21 @@ fn send_and_transfer() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
     runtime.function("step1", Vec::new());
 
     // no receive() required for send/transfer
     assert_eq!(runtime.output(), true.encode());
-    assert_eq!(runtime.balance(2), 1011);
+    assert_eq!(runtime.balance(2), 511);
 
     let mut runtime = build_solidity(
         r##"
         contract c {
             other o;
 
-            constructor() public {
+            constructor() public payable {
                 o = new other();
             }
 
@@ -422,19 +450,21 @@ fn send_and_transfer() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
 
     assert_eq!(runtime.output(), true.encode());
-    assert_eq!(runtime.balance(2), 1011);
+    assert_eq!(runtime.balance(2), 511);
 
     let mut runtime = build_solidity(
         r##"
         contract c {
             other o;
 
-            constructor() public {
+            constructor() public payable {
                 o = new other();
             }
 
@@ -449,17 +479,19 @@ fn send_and_transfer() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
-    assert_eq!(runtime.balance(2), 1011);
+    assert_eq!(runtime.balance(2), 511);
 
     let mut runtime = build_solidity(
         r##"
         contract c {
             other o;
 
-            constructor() public {
+            constructor() public payable {
                 o = new other();
             }
 
@@ -474,9 +506,37 @@ fn send_and_transfer() {
         }"##,
     );
 
+    runtime.set_transferred_value(20000);
     runtime.constructor(0, Vec::new());
 
+    runtime.set_transferred_value(0);
     runtime.function("step1", Vec::new());
 
-    assert_eq!(runtime.balance(2), 1011);
+    assert_eq!(runtime.balance(2), 511);
+}
+
+#[test]
+fn nonpayable_constructor_reverts() {
+    let mut runtime = build_solidity(
+        r#"contract C {
+        uint8 public c;
+        constructor (uint8 val) {
+            c = val;
+        }
+    }"#,
+    );
+
+    let mut input = runtime.blobs()[0].constructors[0].to_vec();
+    let storage_value = 123;
+    input.push(storage_value);
+
+    // Expect the deploy to fail with value
+    runtime.set_transferred_value(1);
+    runtime.raw_constructor_failure(input.clone());
+
+    // The same input should work without value
+    runtime.set_transferred_value(0);
+    runtime.raw_constructor(input.clone());
+    runtime.function("c", Vec::new());
+    assert_eq!(runtime.output(), storage_value.encode());
 }