Remove Default implementation for AccountId

CHANGELOG.md

@@ -6,11 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 - Rename `_checked` codegen call methods with `try_` ‒ [#1621](https://github.com/paritytech/ink/pull/1621)
+- Remove `Default` implementation for AccountId ‒ [#1255](https://github.com/paritytech/ink/pull/1255)
 
 ### Breaking Changes
 
 1. We've renamed some of the generated message methods on the `ContractRef` struct. They
    have been changed from `_checked` to `try_` ([#1621](https://github.com/paritytech/ink/pull/1621))
+1. We have removed the `Default` implementation for `AccountId`s. This is because of
+   security concerns around the use of the zero address which has a known private key in
+   the `sr25519` and `ed25519` curves ([#1255](https://github.com/paritytech/ink/pull/1255)).
 
 ## Version 4.0.0-beta.1
 The coolest feature included in this release is the first first published version of

crates/env/src/call/call_builder.rs

@@ -71,8 +71,10 @@ where
     E: Environment,
 {
     /// Returns the account ID of the called contract instance.
+    ///
+    /// Returns `None` if no account ID has been set for the call.
     #[inline]
-    pub fn callee(&self) -> &E::AccountId {
+    pub fn callee(&self) -> &Option<E::AccountId> {
         &self.call_type.callee
     }
 
@@ -341,7 +343,7 @@ where
 /// The default call type for cross-contract calls. Performs a cross-contract call to `callee`
 /// with gas limit `gas_limit`, transferring `transferred_value` of currency.
 pub struct Call<E: Environment> {
-    callee: E::AccountId,
+    callee: Option<E::AccountId>,
     gas_limit: Gas,
     transferred_value: E::Balance,
 }
@@ -370,7 +372,7 @@ where
     /// Sets the `callee` for the current cross-contract call.
     pub fn callee(self, callee: E::AccountId) -> Self {
         Call {
-            callee,
+            callee: Some(callee),
             gas_limit: self.gas_limit,
             transferred_value: self.transferred_value,
         }
@@ -528,7 +530,7 @@ where
         let call_type = self.call_type.value();
         CallBuilder {
             call_type: Set(Call {
-                callee,
+                callee: Some(callee),
                 gas_limit: call_type.gas_limit,
                 transferred_value: call_type.transferred_value,
             }),

crates/env/src/engine/on_chain/impls.rs

@@ -182,21 +182,6 @@ impl EnvInstance {
         ScopedBuffer::from(&mut self.buffer[..])
     }
 
-    /// Returns the contract property value into the given result buffer.
-    ///
-    /// # Note
-    ///
-    /// This skips the potentially costly decoding step that is often equivalent to a `memcpy`.
-    #[inline(always)]
-    fn get_property_inplace<T>(&mut self, ext_fn: fn(output: &mut &mut [u8])) -> T
-    where
-        T: Default + AsMut<[u8]>,
-    {
-        let mut result = T::default();
-        ext_fn(&mut result.as_mut());
-        result
-    }
-
     /// Returns the contract property value from its little-endian representation.
     ///
     /// # Note
@@ -372,7 +357,8 @@ impl EnvBackend for EnvInstance {
 
 impl TypedEnvBackend for EnvInstance {
     fn caller<E: Environment>(&mut self) -> E::AccountId {
-        self.get_property_inplace::<E::AccountId>(ext::caller)
+        self.get_property::<E::AccountId>(ext::caller)
+            .expect("The executed contract must have a caller with a valid account id.")
     }
 
     fn transferred_value<E: Environment>(&mut self) -> E::Balance {
@@ -388,7 +374,8 @@ impl TypedEnvBackend for EnvInstance {
     }
 
     fn account_id<E: Environment>(&mut self) -> E::AccountId {
-        self.get_property_inplace::<E::AccountId>(ext::address)
+        self.get_property::<E::AccountId>(ext::address)
+            .expect("A contract being executed must have a valid account id.")
     }
 
     fn balance<E: Environment>(&mut self) -> E::Balance {
@@ -425,7 +412,11 @@ impl TypedEnvBackend for EnvInstance {
     {
         let mut scope = self.scoped_buffer();
         let gas_limit = params.gas_limit();
-        let enc_callee = scope.take_encoded(params.callee());
+        let callee = params
+            .callee()
+            .as_ref()
+            .expect("An account ID must be set in order to call a contract.");
+        let enc_callee = scope.take_encoded(callee);
         let enc_transferred_value = scope.take_encoded(params.transferred_value());
         let call_flags = params.call_flags();
         let enc_input = if !call_flags.forward_input() && !call_flags.clone_input() {

crates/env/src/types.rs

@@ -101,16 +101,15 @@ pub trait Environment {
     /// The value must match the maximum number of supported event topics of the used runtime.
     const MAX_EVENT_TOPICS: usize;
 
-    /// The address type.
+    /// The account id type.
     type AccountId: 'static
         + scale::Codec
         + Clone
         + PartialEq
         + Eq
         + Ord
         + AsRef<[u8]>
-        + AsMut<[u8]>
-        + Default;
+        + AsMut<[u8]>;
 
     /// The type of balances.
     type Balance: 'static

crates/primitives/src/types.rs

@@ -28,18 +28,7 @@ use scale_info::TypeInfo;
 /// This is a mirror of the `AccountId` type used in the default configuration
 /// of PALLET contracts.
 #[derive(
-    Debug,
-    Copy,
-    Clone,
-    PartialEq,
-    Eq,
-    Ord,
-    PartialOrd,
-    Hash,
-    Encode,
-    Decode,
-    From,
-    Default,
+    Debug, Copy, Clone, PartialEq, Eq, Ord, PartialOrd, Hash, Encode, Decode, From,
 )]
 #[cfg_attr(feature = "std", derive(TypeInfo))]
 pub struct AccountId([u8; 32]);

examples/dns/lib.rs

@@ -53,7 +53,6 @@ mod dns {
     /// to facilitate transfers, voting and DApp-related operations instead
     /// of resorting to long IP addresses that are hard to remember.
     #[ink(storage)]
-    #[derive(Default)]
     pub struct DomainNameService {
         /// A hashmap to store all name to addresses mapping.
         name_to_address: Mapping<Hash, AccountId>,
@@ -63,6 +62,21 @@ mod dns {
         default_address: AccountId,
     }
 
+    impl Default for DomainNameService {
+        fn default() -> Self {
+            let mut name_to_address = Mapping::new();
+            name_to_address.insert(Hash::default(), &zero_address());
+            let mut name_to_owner = Mapping::new();
+            name_to_owner.insert(Hash::default(), &zero_address());
+
+            Self {
+                name_to_address,
+                name_to_owner,
+                default_address: zero_address(),
+            }
+        }
+    }
+
     /// Errors that can occur upon calling this contract.
     #[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
     #[cfg_attr(feature = "std", derive(::scale_info::TypeInfo))]
@@ -165,6 +179,13 @@ mod dns {
         }
     }
 
+    /// Helper for referencing the zero address (`0x00`). Note that in practice this address should
+    /// not be treated in any special way (such as a default placeholder) since it has a known
+    /// private key.
+    fn zero_address() -> AccountId {
+        [0u8; 32].into()
+    }
+
     #[cfg(test)]
     mod tests {
         use super::*;

examples/erc1155/lib.rs

@@ -432,7 +432,7 @@ mod erc1155 {
                 ensure!(self.is_approved_for_all(from, caller), Error::NotApproved);
             }
 
-            ensure!(to != AccountId::default(), Error::ZeroAddressTransfer);
+            ensure!(to != zero_address(), Error::ZeroAddressTransfer);
 
             let balance = self.balance_of(from, token_id);
             ensure!(balance >= value, Error::InsufficientBalance);
@@ -457,7 +457,7 @@ mod erc1155 {
                 ensure!(self.is_approved_for_all(from, caller), Error::NotApproved);
             }
 
-            ensure!(to != AccountId::default(), Error::ZeroAddressTransfer);
+            ensure!(to != zero_address(), Error::ZeroAddressTransfer);
             ensure!(!token_ids.is_empty(), Error::BatchTransferMismatch);
             ensure!(
                 token_ids.len() == values.len(),
@@ -581,6 +581,13 @@ mod erc1155 {
         }
     }
 
+    /// Helper for referencing the zero address (`0x00`). Note that in practice this address should
+    /// not be treated in any special way (such as a default placeholder) since it has a known
+    /// private key.
+    fn zero_address() -> AccountId {
+        [0u8; 32].into()
+    }
+
     #[cfg(test)]
     mod tests {
         /// Imports all the definitions from the outer scope so we can use them here.