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

Issue with executeOrder function #23

Closed neocybereth closed 5 years ago

neocybereth commented 6 years ago
const marketplace = new contracts.Marketplace("0xb3bca6f5052c7e24726b44da7403b56a8a1b98f8")

async function purchase(assetId, price) {
  await eth.connect({
    provider: 'https://mainnet.infura.io/{credentials}',
    contracts: [
      marketplace
    ]
  })
  .catch(console.error)
  await marketplace.executeOrder(assetId, eth.utils.toWei(price))
}

getCurrentProperties()
.then((properties) => {
    console.log(properties[0].asset_id, properties[0].price)
        purchase(properties[0].asset_id, properties[0].price)
        .catch(console.error)           
})
.catch(console.error)

When trying to use the executeOrder function I'm running into this error when passing in my assetId:

BigNumber Error: new BigNumber() not a number: 24,150

Any ideas what could be causing this error?

menduz commented 6 years ago

@NicoSantangelo @abarmat

nicosantangelo commented 6 years ago

@Sm00g15 I think it's related to web3 expecting a valid number and getting something that it can't parse.

To reproduce it I did the following:

eth.utils.toWei('12,333')
// This throws:
// BigNumber Error: new BigNumber() not a number: 12,333

The Typescript signature of the method is number|BigNumber, to avoid this very problem, you need to cast it before feeding it to toWei

neocybereth commented 6 years ago

What do you mean by cast it? I’m not familiar

On 23/07/2018, at 11:26 PM, Nicolás Santángelo notifications@github.com wrote:

@Sm00g15 I think it's related to web3 expecting a valid number and getting something that it can't parse.

To reproduce it I did the following:

eth.utils.toWei('12,333') // This throws: // BigNumber Error: new BigNumber() not a number: 12,333 The Typescript signature of the method is number|BigNumber, to avoid this very problem, you need to cast it before feeding it to toWei

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

nicosantangelo commented 6 years ago

Sorry!

I mean changing the type, in this case from string to number. For example:

typeof '12,333' => string
typeof 12333 => number

Number('12,333') => NaN (not a number)
Number('12333') => 12333
Number('12.333') => 12.333

so if I have the string '12,333' but I want to represent the number 12333 a quick solution would be doing:

let price = '12,333'
price = price.replace(/,/g, '')
Number(price) // 12333

is that a bit more clear?

neocybereth commented 6 years ago

yeah, that definitely makes sense, thank you!

Catch is when I run that address in the executeOrder function I get:

Error: invalid address
    at inputAddressFormatter (/Users/smagin07/Desktop/dcl/node_modules/decentraland-eth/node_modules/web3/lib/web3/formatters.js:274:11)

For more context, here's the issue I'm trying to solve (my concerns are about halfway down the thread):

https://github.com/decentraland/marketplace/issues/294

EDIT: After recreating the same code listed by @cazala I was able to successfully complete a transaction using the React code, just not the vanilla JS code...weird! Anyway, thank you @cazala and @NicoSantangelo

neocybereth commented 6 years ago

@NicoSantangelo Do you know how I would handle coordinates with a - in the executeOrder function?

like for instance: -25, -133 OR -25, 133 OR 25, -133

cazala commented 6 years ago

You can use the LANDRegistry contract to convert x,y coords into asset ids:

import { eth, contracts } from 'decentraland-eth'

const landRegistryContract = new contracts.LANDRegistry('0xf87e31492faf9a91b02ee0deaad50d51d56d5d4d')
const marketplaceContract = new contracts.Marketplace('0xb3bca6f5052c7e24726b44da7403b56a8a1b98f8')

async function main() {
  await eth.connect({
    provider: 'http://localhost:8545',
    contracts: [
      landRegistryContract,
      marketplaceContract
    ]
  })

  const assetId = await landRegistryContract.encodeTokenId(-25, -133)
  const price = eth.utils.toWei(10000) // 10K MANA
  await marketplaceContract.executeOrder(assetId, price)
}

main()
cazala commented 6 years ago

Same way you can convert assetIds to coords via the LANDRegistry contract:

const coords = await landRegistryContract.decodeTokenId(assetId)
neocybereth commented 6 years ago

Awesomeeee, thanks @cazala. Is there any place to find all the available methods of decentraland-eth besides looking in the ABI? I feel bad having to ping you guys all the time, I know you've gotta be super busy!!!

cazala commented 6 years ago

Sadly no :( we used to have some sort of auto-generated docs but they are broken now.

But I think there's a better option than reading thru the ABIs and is using etherscan.io

Here you have all the addresses for all our contracts: https://contracts.decentraland.org/addresses.json

You can use etherscan to see all the available methods and signatures, for instance these are all the marketplace read methods, and these are all the write methods

There's one important thing to know about two addresses in that list. You will see that there's a LANDRegistry and a LANDProxy. You should always use the LANDProxy address in your code, because it's an upgradable contract. So we might deploy a new version in the future, like a bug fix or a new feature, and the address of LANDRegistry will be a different one, but the LANDProxy address will never change. If you see at the code I posted in the comment above:

const landRegistryContract = new contracts.LANDRegistry('0xf87e31492faf9a91b02ee0deaad50d51d56d5d4d')

☝️ That's using the LANDProxy address from this list.

cazala commented 6 years ago

I feel bad having to ping you guys all the time, I know you've gotta be super busy!!!

No worries, I'm happy every time I see people from outside our team using the tools we make (: I'm glad to help!