f-o-a-m / purescript-web3

a purescript library for the web3 api
Apache License 2.0
127 stars 24 forks source link

Add a phantom type to `Address` (in ps-eth-core) and TransactionOptions in ps-web3 #129

Closed iostat closed 5 months ago

iostat commented 5 years ago

Currently, Address is just a plain old type that can be passed around, and every ps-web3-generated contract call function just takes a TransactionOptions Payable/NoPay as its first argument. While this is really convenient, it also leads to potential for grossly incorrect code that still passes typechecking and compiles. For example,

import TheKitchenSink -- all necessary types, ps-web3, etc.
import Contracts.SomeERC20  as ERC20
import Contracts.SomeERC721 as ERC721

-- it just so happened that the ERC20 and ERC721 contracts both had the
-- arguments of their transferFrom() function as transferFrom(address from, address to, uint256 value)
doAThing :: Address -> Address -> Address -> Address -> Web3 Unit
doAThing userAddress otherUserAddress erc20Address erc721Address = do
  let erc20Options  = defaultTransactionOptions # _to ?~ erc20Address
      erc721Options = defaultTransactionOptions # _to ?~ erc721Address
  void $ ERC20.transferFrom  erc20Options { from: userAddress, to: erc721Address, value: embed 12345 }
  void $ ERC721.transferFrom erc20Options { from: erc20Address, to: userAddress, value: embed 12345 }
  void $ ERC721.transferFrom erc721Options { from: otherUserAddress, to: userAddress, value: embed 12345 }
  void $ ERC20.transferFrom  erc721Options { from: erc721Address, to: otherUserAddress, value: embed 12345 }

In this example, I can use TransactionOptions that send to a SomeERC20 contract while actually calling an function intended for a SomeERC721 contract, with nothing stopping me, and vice versa. Moreover, I can't determine the order of the arguments to doAThing from the type alone. It gets even worse -- as the names of the parameters suggest, I wanted userAddress and otherUserAddress to be two different addresses, but, there's nothing stopping me from enforcing that userAddress and otherUserAddress are actually different addresses!

On the other hand, by adding a phantom type, I can write doAThing in much more concrete and typesafe ways. For example, if I could write a signature like

import Type.Data.Boolean (False)
import Type.IsEqual (class IsEqual)
doAThing
  :: forall user otherUser
   . IsEqual user otherUser False
  => Address user
  -> Address otherUser
  -> Address SomeERC20Contract
  -> Address SomeERC721Contract
  -> Web3 Unit

I can now make my intentions very clear as to what the Addresses are supposed to be, in addition to having the compiler enforce usage (a user of doAThing would need to construct Address user and Address otherUser making room for error much lower -- yes they can still do two separate mkAddresses but at least that's less likely to accidentally happen). Of course, this now means TransactionOptions tokenUnit has to become TransactionOptions tokenUnit addressType (more correctly -- TransactionOptions tokenUnit fromAddressType toAddressType -- fromAddressType will be omitted for brevity), which is actually a great thing! The signatures of transferFrom can now change as well, making it much more difficult for me to call the ERC721 contract with an ERC20 address and vice-versa (again, i'd have to construct the address more explictly):

-- before:
module Contracts.SomeERC20 where
transferFrom :: TransactionOptions NoPay -> { from :: Address, to :: Address, value :: Uint256 } -> Web3 TransactionHash

module Contracts.SomeERC721 where
transferFrom :: TransactionOptions NoPay -> { from :: Address, to :: Address, value :: Uint256} -> Web3 TransactionHash

-- after:
module Contracts.SomeERC20 where
foreign import data SomeERC20Address :: Type
transferFrom :: forall from to. TransactionOptions NoPay SomeERC20Address -> { from :: Address from, to :: Address to, value :: Uint256 } -> Web3 TransactionHash

module Contracts.SomeERC721 where
foreign import data SomeERC721Address :: Type
transferFrom :: forall from to. TransactionOptions NoPay SomeERC721Address -> { from :: Address from, to :: Address to, value: Uint256 } -> Web3 TransactionHash

Later on down the line, we could potentially have purescript-web3-generator read AST output from solc to determine exactly what type of address each argument is, for instance, a TCR contract could go from

module Contracts.TCR where
constructor :: TransactionOptions NoPay -> { plcr :: Address, params :: Address } -> Web3 TransactionHash

to

module Contracts.TCR where
import Contracts.PLCRVoting (PLCRVotingAddress)
import Contracts.Parameterizer (ParameterizerAddress)
import Network.Ethereum.Types (Address, ZeroAddress)
constructor :: TransactionOptions NoPay ZeroAddress -> { plcr :: Address PLCRVotingAddress, params :: Address ParameterizerAddress } -> Web3 TransactionHash

This also has the potential to be useful in Chanterelle's library WIP, where I can force a deployment script to deploy and link the correct library (a Library can still have PureScript code generated for the foreign import data, forcing a user to deploy or otherwise supply a library address to linkLibrary of the correct library type, reducing reliance on meticulous double-checking of code.

This is naturally a very breaking change, so I propose this for a purescript-web3 2.0 eventually.

iostat commented 5 years ago

@blinky3713 @safareli thoughts and comments would be highly appreciated

safareli commented 5 years ago

I suspect it's expected this functions to be generated by the web3 generator:

module Contracts.TCR where
constructor :: TransactionOptions NoPay ZeroAddress -> { plcr :: Address PLCRVotingAddress, params :: Address ParameterizerAddress } -> Web3 TransactionHash

in which case, It's not quite clear to how we are going to go from:

  constructor(
        address _plcrAddr,
        address _paramsAddr
  )

to

{ plcr :: Address PLCRVotingAddress
, params :: Address ParameterizerAddress 
}
iostat commented 5 years ago

Actually rethinking this a little bit, it makes more sense to have purescript-web3 declare its own Address a type over purescript-eth-core's address, as that's a semantic introduced by ps-web3 and currently ps-web3 just reexport's ps-eth-core's Address type.

@safareli Initially, the codegen can simply generate forall address1 address2. TxOptions ... -> { plcr :: Address address1, params :: Address address2 } -> Web3 TxHash. Later on, the codegen can enhance solc's ABI output with solc's AST output which gives you the type of each parameter (i.e., you can distinguish whether it's declared as an address or SomeContract in the parameter list) which gives you the type of each parameter. Because Chanterelle expects you to keep a Haskell/PureScript module/contract naming scheme, you can be sure that if the argument is of, say, Parameterizer type, there's going be a ParameterizerAddress generated by the codegen in Parameterizer's PureScript module. Because you are similarly expected to construct the contract with a Chanterelle deploy script, the codegen can import Contracts.Parameterizer (or whatever the configured module prefix is) in Contracts.TCR with the assumption that Parameterizer is being codegen'd independently as part of Chanterelle invoking it.