f-o-a-m / purescript-web3

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

v0.14 #146

Closed kejace closed 3 years ago

kejace commented 3 years ago

Attempt to address #145

Currently getting "real" errors, as upstream dependencies have been fixed. I have a PR for purescript-eth-core (https://github.com/f-o-a-m/purescript-eth-core/pull/24) that passes tests. I also had to fork purescript-tagged (which had a similar issue to below). I think we should remove the purescript-tagged dependency, but leaving that outside the scope of this PR.

Some kind annotations have been added, to guide the type-checker.

Currently running into

Compiling Network.Ethereum.Web3.Solidity.Generic
Compiling Network.Ethereum.Web3.Solidity.Event
Error found:
in module Network.Ethereum.Web3.Solidity.Event
at src/Network/Ethereum/Web3/Solidity/Event.purs:89:29 - 89:35 (line 89, column 29 - line 89, column 35)

  No type class instance was found for

    Prim.Coerce.Coercible cfieldsRes4
                          t2         

  The instance head contains unknown type variables. Consider adding a type annotation.

while solving type class constraint

  Prim.Coerce.Coercible (Record t2)
                        c3         

while checking that type forall (a :: Type) (b :: Type). Coercible @Type a b => a -> b
  is at least as general as type t0 -> t1
while checking that expression coerce
  has type t0 -> t1
in value declaration combineChange

where c3 is a rigid type variable
        bound at (line 0, column 0 - line 0, column 0)
      cfieldsRes4 is a rigid type variable
        bound at (line 0, column 0 - line 0, column 0)
      t0 is an unknown type
      t1 is an unknown type
      t2 is an unknown type

See https://github.com/purescript/documentation/blob/master/errors/NoInstanceFound.md for more information,
or to contribute content related to this error.

The change is basically

+ combineChange (Event a b) = coerce $ build (merge (genericToRecordFields a)) (genericToRecordFields b)
- combineChange (Event a b) = wrap $ build (merge (genericToRecordFields a)) (genericToRecordFields b)

but it looks like the constraints aren't quite right anymore.

The reason for this change is due to the breaking changes regarding the removal of wrap and unwrap for Newtype and addition the Coercible typeclass.

Scroll down to the relevant section here: https://github.com/purescript/purescript/releases/tag/v0.14.0 and also read https://github.com/purescript/purescript-newtype/pull/22


EDIT: I resolved this by using unsafeCoerce for now, and re-arranging the terms in the Row.Union constraint.

kejace commented 3 years ago

I think I have it down to just fixing the instance resolution for event'.

kejace commented 3 years ago

This passes CI, requesting review.

All above issues are gone - no unsafeCoerce is present and tests pass.

Some comments:

  1. had clash between TokenUnit as kind and class. Therefore opted to rename the kind to just TokenK as it is not really exported (it is, but only because we need to for divide).
  2. we still depend on purescript-tagged for which I have opened a PR to upstream. I leave it to the reviewer what to do about waiting for that to merge or not, or just not use it at all.
  3. we currently use the v0.14 branch of eth-core but this should change whenever there's a merge of that to master.
kejace commented 3 years ago

purty fails because it doesn't fully support class kind annotations

kejace commented 3 years ago

Disabled linting, for now

martyall commented 3 years ago

We have a heavy dependency on purescript-tagged via the generator, so I don't think we can/should get rid of it. Its strange to me that it's fallen out of maintenance because it's often a useful type in haskell.

martyall commented 3 years ago

Also are the chanterelle tests in this repo running? I can't tell just by looking at it

kejace commented 3 years ago

Also are the chanterelle tests in this repo running? I can't tell just by looking at it

I'm not sure what you are referring to here. FWIW the updated chanterelle which depends on this branch passes tests. I don't see any dependency in this repo, on chanterelle itself.