search

perun-network / perun-eth-payment

Simplified payment channel API of go-perun.
Apache License 2.0
5 stars 1 forks source link

Don't use abstract go-perun types like `wallet.Address` in the API #9

Open sebastianst opened 3 years ago

sebastianst commented 3 years ago

This is an eth-specific wrapper, so it should hide abstractions. Client.OnChainBalance currently uses wallet.Address in its parameters. Didn't check if there are other abstract types visible that could be concretized.

ggwpez commented 3 years ago

I replaced it with a common.Address.
We also use wallet.Address and wire.Address in some cases. They could all be changed/should all be changed to common.Address. This would require some wrapper code.

PS: These changes should be done all at once to prevent inconsistency. Otherwise the user would need to cast between wallet.Address, wire.Address and common.Address.

matthiasgeihs commented 3 years ago

To be honest, I don't immediately see the benefit of using wallet.Address over common.Address.

@sebastianst Can you give an example?

sebastianst commented 3 years ago

One of the purposes of this eth payment api is to hide abstract types like wallet.Address. It is just confusing for the user, particularly because most of the api already uses common.Address.

sebastianst commented 3 years ago

Right now, the API mixes up the use of concrete types like common.Address and the abstract wallet.Address, which is not clean. A new user would be confused how to even pass a common.Address for this wallet.Address, because they would need to find out how to convert it.

Moreover, a function should only ask in its parameters for what it can handle. And the functions in this package can actually only handle common.Addresses and not any other implementation of wallet.Address, so it provides more type safety to accept only the concrete type.