decentraland / decentraland-eth

DEPRECATED - Ethereum common helpers for Decentraland
https://decentraland.github.io/decentraland-eth/
Apache License 2.0
15 stars 9 forks source link

feat: send tx by type #42

Closed nachomazzara closed 5 years ago

nachomazzara commented 5 years ago

Ability to send transaction by type when there is an overloading with function names

Usage

Send Call

const balance = await MANAFacade.balanceOf(
  account
)

// Or
const balance = await MANAFacade.sendCallByType('balanceOf', 'address', account)

// Or
const balance = await MANAFacade.balanceOf['address'](
  account
)

Send Transaction

const balance = await MANAFacade.mint(
  account, 
  amount
)

// Or
const balance = await MANAFacade.sendTransactionByType(
  'mint', 
  'address,uint256', 
  account, 
  amount
)

// Or
const balance = await MANAFacade.mint['address,uint256']( 
  account, 
  amount
)

Note

Also added Marketplace-Contract ABI with overloaded events

menduz commented 5 years ago

please add test

nachomazzara commented 5 years ago

please add test

That's why it is WIP

menduz commented 5 years ago

🔥

nachomazzara commented 5 years ago

@menduz @NicoSantangelo test added!

menduz commented 5 years ago

Looks good. But

I'm starting feeling we should not add any other method to Contract.ts and instead start creating functions that receive the contract. Like

await eth.callContract(ContractInstance, 'balanceOf', address)
await ContractInstance.balanceOf(address) // keep some overload in the contract

await eth.sendContractTransaction(ContractInstance, 'mint', address, amount)
await eth.sendContractTransaction(ContractInstance, 'mint', 'address,uint256', address, amount)
await ContractInstance.mint(address, amount) // keep some overload in the contract

I'm also in doubt about which overload should be the default in the contract. Lets assume we have

contract X {
  mint(a: address, b: uint256) {...}
  mint(a: address) {...}
}

Then we have the

function callMint(){
   await XInstance.mint.apply(...arguments)
}
nachomazzara commented 5 years ago

@menduz thanks for the review!

Context about the usage.

We support the legacy methods for the marketplace-contract which has the same name but different parameters.

E.g:

// Cancel Order
function cancelOrder(address nftAddress, uint256 assetId);

// Cancel Order Legacy
function cancelOrder(uint256 assetId);

Nowadays is not a problem because we can manipulate the ABI and remove the legacy methods cause at the marketplace Dapp we don't need the legacy ones, but would be great to stop removing methods from ABI and select the method from our Dapps by type