From c7c9f9ddf65bbb6af8c1f5bb0645019506f58c77 Mon Sep 17 00:00:00 2001
From: Valentin <77051586+vkgnosis@users.noreply.github.com>
Date: Thu, 11 Mar 2021 13:20:04 +0100
Subject: [PATCH] Remove Return and Void trait

We no longer need them because we represent no return type as an empty
tuple instead of a placeholder type.
---
 ethcontract-generate/src/contract/methods.rs | 10 +--
 ethcontract/src/contract.rs                  |  8 +-
 ethcontract/src/contract/method.rs           | 81 +++++---------------
 ethcontract/src/lib.rs                       |  4 +-
 ethcontract/src/tokens.rs                    |  8 +-
 5 files changed, 33 insertions(+), 78 deletions(-)

diff --git a/ethcontract-generate/src/contract/methods.rs b/ethcontract-generate/src/contract/methods.rs
index bafffa89..467344ec 100644
--- a/ethcontract-generate/src/contract/methods.rs
+++ b/ethcontract-generate/src/contract/methods.rs
@@ -145,7 +145,7 @@ pub(crate) fn expand_inputs_call_arg(inputs: &[Param]) -> TokenStream {
 
 fn expand_fn_outputs(outputs: &[Param]) -> Result<TokenStream> {
     match outputs.len() {
-        0 => Ok(quote! { self::ethcontract::Void }),
+        0 => Ok(quote! { () }),
         1 => types::expand(&outputs[0].kind),
         _ => {
             let types = outputs
@@ -170,9 +170,7 @@ fn expand_fallback(cx: &Context) -> TokenStream {
             impl Contract {
                 /// Returns a method builder to setup a call to a smart
                 /// contract's fallback function.
-                pub fn fallback<D>(&self, data: D) -> self::ethcontract::dyns::DynMethodBuilder<
-                    self::ethcontract::Void,
-                >
+                pub fn fallback<D>(&self, data: D) -> self::ethcontract::dyns::DynMethodBuilder<()>
                 where
                     D: Into<Vec<u8>>,
                 {
@@ -219,9 +217,7 @@ mod tests {
 
     #[test]
     fn expand_fn_outputs_empty() {
-        assert_quote!(expand_fn_outputs(&[],).unwrap(), {
-            self::ethcontract::Void
-        });
+        assert_quote!(expand_fn_outputs(&[],).unwrap(), { () });
     }
 
     #[test]
diff --git a/ethcontract/src/contract.rs b/ethcontract/src/contract.rs
index fa95393f..8553a6b7 100644
--- a/ethcontract/src/contract.rs
+++ b/ethcontract/src/contract.rs
@@ -25,7 +25,7 @@ pub use self::event::{
     AllEventsBuilder, Event, EventBuilder, EventMetadata, EventStatus, ParseLog, RawLog,
     StreamEvent, Topic,
 };
-pub use self::method::{MethodBuilder, MethodDefaults, Return, ViewMethodBuilder, Void};
+pub use self::method::{MethodBuilder, MethodDefaults, ViewMethodBuilder};
 
 /// Represents a contract instance at an address. Provides methods for
 /// contract interaction.
@@ -166,7 +166,7 @@ impl<T: Transport> Instance<T> {
     pub fn method<P, R>(&self, signature: H32, params: P) -> AbiResult<MethodBuilder<T, R>>
     where
         P: Tokenize,
-        R: Return,
+        R: Tokenize,
     {
         let signature = signature.as_ref();
         let function = self
@@ -198,7 +198,7 @@ impl<T: Transport> Instance<T> {
     pub fn view_method<P, R>(&self, signature: H32, params: P) -> AbiResult<ViewMethodBuilder<T, R>>
     where
         P: Tokenize,
-        R: Return,
+        R: Tokenize,
     {
         Ok(self.method(signature, params)?.view())
     }
@@ -208,7 +208,7 @@ impl<T: Transport> Instance<T> {
     ///
     /// This method will error if the ABI does not contain an entry for a
     /// fallback function.
-    pub fn fallback<D>(&self, data: D) -> AbiResult<MethodBuilder<T, Void>>
+    pub fn fallback<D>(&self, data: D) -> AbiResult<MethodBuilder<T, ()>>
     where
         D: Into<Vec<u8>>,
     {
diff --git a/ethcontract/src/contract/method.rs b/ethcontract/src/contract/method.rs
index 34ea475c..2351f4ed 100644
--- a/ethcontract/src/contract/method.rs
+++ b/ethcontract/src/contract/method.rs
@@ -14,52 +14,6 @@ use web3::types::{Address, BlockId, Bytes, CallRequest, U256};
 use web3::Transport;
 use web3::{api::Web3, BatchTransport};
 
-/// A solidity return type. We need a separate trait for this so that we can represent functions
-/// that return nothing.
-pub trait Return {
-    /// The output type.
-    type Output: Tokenize;
-    /// Is this the void return type?
-    fn is_void() -> bool;
-    /// Convert tokens into self.
-    fn from_tokens(tokens: Vec<Token>) -> Result<Self::Output, TokenError>;
-}
-
-/// The return type of a solidity function that returns nothing.
-pub struct Void;
-impl Return for Void {
-    type Output = ();
-
-    fn is_void() -> bool {
-        true
-    }
-
-    fn from_tokens(tokens: Vec<Token>) -> Result<Self::Output, TokenError> {
-        if !tokens.is_empty() {
-            return Err(TokenError::MultiDetokenizeLengthMismatch);
-        }
-        Ok(())
-    }
-}
-
-impl<T> Return for T
-where
-    T: Tokenize,
-{
-    type Output = T;
-
-    fn is_void() -> bool {
-        false
-    }
-
-    fn from_tokens(tokens: Vec<Token>) -> Result<Self::Output, TokenError> {
-        if tokens.len() != 1 {
-            return Err(TokenError::MultiDetokenizeLengthMismatch);
-        }
-        Self::Output::from_token(tokens.into_iter().next().unwrap())
-    }
-}
-
 /// Default options to be applied to `MethodBuilder` or `ViewMethodBuilder`.
 #[derive(Clone, Debug, Default)]
 pub struct MethodDefaults {
@@ -76,7 +30,7 @@ pub struct MethodDefaults {
 /// transactions. This is useful when dealing with view functions.
 #[derive(Debug, Clone)]
 #[must_use = "methods do nothing unless you `.call()` or `.send()` them"]
-pub struct MethodBuilder<T: Transport, R: Return> {
+pub struct MethodBuilder<T: Transport, R: Tokenize> {
     web3: Web3<T>,
     function: Function,
     /// transaction parameters
@@ -84,7 +38,7 @@ pub struct MethodBuilder<T: Transport, R: Return> {
     _result: PhantomData<R>,
 }
 
-impl<T: Transport> MethodBuilder<T, Void> {
+impl<T: Transport> MethodBuilder<T, ()> {
     /// Creates a new builder for a transaction invoking the fallback method.
     pub fn fallback(web3: Web3<T>, address: Address, data: Bytes) -> Self {
         // NOTE: We create a fake `Function` entry for the fallback method. This
@@ -102,7 +56,7 @@ impl<T: Transport> MethodBuilder<T, Void> {
     }
 }
 
-impl<T: Transport, R: Return> MethodBuilder<T, R> {
+impl<T: Transport, R: Tokenize> MethodBuilder<T, R> {
     /// Creates a new builder for a transaction.
     pub fn new(web3: Web3<T>, function: Function, address: Address, data: Bytes) -> Self {
         MethodBuilder {
@@ -188,7 +142,7 @@ impl<T: Transport, R: Return> MethodBuilder<T, R> {
     /// as such do not require gas or signing. Note that doing a call with a
     /// block number requires first demoting the `MethodBuilder` into a
     /// `ViewMethodBuilder` and setting the block number for the call.
-    pub async fn call(self) -> Result<R::Output, MethodError> {
+    pub async fn call(self) -> Result<R, MethodError> {
         self.view().call().await
     }
 }
@@ -197,14 +151,14 @@ impl<T: Transport, R: Return> MethodBuilder<T, R> {
 /// directly send transactions and is for read only method calls.
 #[derive(Debug, Clone)]
 #[must_use = "view methods do nothing unless you `.call()` them"]
-pub struct ViewMethodBuilder<T: Transport, R: Return> {
+pub struct ViewMethodBuilder<T: Transport, R: Tokenize> {
     /// method parameters
     pub m: MethodBuilder<T, R>,
     /// optional block number
     pub block: Option<BlockId>,
 }
 
-impl<T: Transport, R: Return> ViewMethodBuilder<T, R> {
+impl<T: Transport, R: Tokenize> ViewMethodBuilder<T, R> {
     /// Create a new `ViewMethodBuilder` by demoting a `MethodBuilder`.
     pub fn from_method(method: MethodBuilder<T, R>) -> Self {
         ViewMethodBuilder {
@@ -254,10 +208,10 @@ impl<T: Transport, R: Return> ViewMethodBuilder<T, R> {
     }
 }
 
-impl<T: Transport, R: Return> ViewMethodBuilder<T, R> {
+impl<T: Transport, R: Tokenize> ViewMethodBuilder<T, R> {
     /// Call a contract method. Contract calls do not modify the blockchain and
     /// as such do not require gas or signing.
-    pub async fn call(self) -> Result<R::Output, MethodError> {
+    pub async fn call(self) -> Result<R, MethodError> {
         let eth = &self.m.web3.eth();
         let (function, call, block) = self.decompose();
         let future = eth.call(call, block);
@@ -270,7 +224,7 @@ impl<T: Transport, R: Return> ViewMethodBuilder<T, R> {
     pub fn batch_call<B: BatchTransport>(
         self,
         batch: &mut CallBatch<B>,
-    ) -> impl std::future::Future<Output = Result<R::Output, MethodError>> {
+    ) -> impl std::future::Future<Output = Result<R, MethodError>> {
         let (function, call, block) = self.decompose();
         let future = batch.push(call, block);
         async move { convert_response::<_, R>(future, function).await }
@@ -294,11 +248,11 @@ impl<T: Transport, R: Return> ViewMethodBuilder<T, R> {
 
 async fn convert_response<
     F: std::future::Future<Output = Result<Bytes, web3::Error>>,
-    R: Return,
+    R: Tokenize,
 >(
     future: F,
     function: Function,
-) -> Result<R::Output, MethodError> {
+) -> Result<R, MethodError> {
     let bytes = future
         .await
         .map_err(|err| MethodError::new(&function, err))?;
@@ -316,14 +270,15 @@ async fn convert_response<
 /// encode this information in a JSON RPC error. On a revert or invalid opcode,
 /// the result is `0x` (empty data), while on a revert with message, it is an
 /// ABI encoded `Error(string)` function call data.
-fn decode_geth_call_result<R: Return>(
+fn decode_geth_call_result<R: Tokenize>(
     function: &Function,
     bytes: Vec<u8>,
-) -> Result<R::Output, ExecutionError> {
+) -> Result<R, ExecutionError> {
+    let is_void = R::from_token(Token::Tuple(Vec::new())).is_ok();
     if let Some(reason) = revert::decode_reason(&bytes) {
         // This is an encoded revert message from Geth nodes.
         Err(ExecutionError::Revert(Some(reason)))
-    } else if !R::is_void() && bytes.is_empty() {
+    } else if !is_void && bytes.is_empty() {
         // Geth does this on `revert()` without a message and `invalid()`,
         // just treat them all as `invalid()` as generally contracts revert
         // with messages. Unfortunately, for methods with empty return types
@@ -333,7 +288,11 @@ fn decode_geth_call_result<R: Return>(
     } else {
         // just a plain ol' regular result, try and decode it
         let tokens = function.decode_output(&bytes)?;
-        let result = R::from_tokens(tokens)?;
+        let result = match tokens.len() {
+            0 => R::from_token(Token::Tuple(Vec::new())),
+            1 => R::from_token(tokens.into_iter().next().unwrap()),
+            _ => Err(TokenError::WrongNumberOfTokensReturned),
+        }?;
         Ok(result)
     }
 }
diff --git a/ethcontract/src/lib.rs b/ethcontract/src/lib.rs
index 39b5a139..1a33a20e 100644
--- a/ethcontract/src/lib.rs
+++ b/ethcontract/src/lib.rs
@@ -118,9 +118,7 @@ pub mod prelude {
     //! A prelude module for importing commonly used types when interacting with
     //! generated contracts.
 
-    pub use crate::contract::{
-        Event, EventMetadata, EventStatus, RawLog, StreamEvent, Topic, Void,
-    };
+    pub use crate::contract::{Event, EventMetadata, EventStatus, RawLog, StreamEvent, Topic};
     pub use crate::int::I256;
     pub use crate::secret::{Password, PrivateKey};
     pub use crate::transaction::{Account, GasPrice};
diff --git a/ethcontract/src/tokens.rs b/ethcontract/src/tokens.rs
index 9637ad17..80927142 100644
--- a/ethcontract/src/tokens.rs
+++ b/ethcontract/src/tokens.rs
@@ -47,9 +47,11 @@ pub enum Error {
     /// Tokenize::from_token token is tuple with wrong length.
     #[error("expected a different number of tokens in tuple")]
     TupleLengthMismatch,
-    /// MultiDetokenize::from_tokens has wrong number of tokens.
-    #[error("expected a different number of tokens when detokenizing multiple tokens")]
-    MultiDetokenizeLengthMismatch,
+    /// Methods should only ever return 0 or 1 token because multiple return types are grouped into
+    /// into a tuple which is a single token. If a method nonetheless returns more than 1 token this
+    /// error occurs.
+    #[error("a method returned an unexpected number of tokens")]
+    WrongNumberOfTokensReturned,
 }
 
 /// Rust type and single token conversion.