OriginProtocol / origin-js

We've moved to a monorepo: https://github.com/OriginProtocol/origin
MIT License
81 stars 33 forks source link

Event-listener failing to process offer #594

Closed franckc closed 5 years ago

franckc commented 5 years ago

I noticed this error in the event-listener logs on the Mainnet (but it is not mainnet specific, I also saw it on Testnet):

2018-10-03T05:40:57.583192583Z Processing log blockNumber=6440484 transactionIndex=156
2018-10-03T05:41:09.038217808Z INDEXING  1-000-10
2018-10-03T05:41:09.081578188Z Indexed listing 1-000-10 in search index.
2018-10-03T05:41:09.11546347Z Added row 1-000-10 to listing table.
2018-10-03T05:41:09.115530108Z Processing log blockNumber=6440925 transactionIndex=44
2018-10-03T05:41:41.668348381Z Error: could not get information for V00_Marketplace OfferCreated
2018-10-03T05:41:41.673968009Z TypeError: Cannot read property 'toLowerCase' of undefined
2018-10-03T05:41:41.673997345Z     at Marketplace._callee7$ (/app/daemon/indexing/node_modules/origin/dist/index.js:6110:79)
2018-10-03T05:41:41.67400165Z     at tryCatch (/app/daemon/indexing/node_modules/babel-polyfill/node_modules/regenerator-runtime/runtime.js:65:40)
2018-10-03T05:41:41.674005272Z     at Generator.invoke [as _invoke] (/app/daemon/indexing/node_modules/babel-polyfill/node_modules/regenerator-runtime/runtime.js:303:22)
2018-10-03T05:41:41.674008758Z     at Generator.prototype.(anonymous function) [as next] (/app/daemon/indexing/node_modules/babel-polyfill/node_modules/regenerator-runtime/runtime.js:117:21)
2018-10-03T05:41:41.674012204Z     at step (/app/daemon/indexing/node_modules/origin/dist/index.js:5771:191)
2018-10-03T05:41:41.674015253Z     at /app/daemon/indexing/node_modules/origin/dist/index.js:5771:361
2018-10-03T05:41:41.674026379Z     at <anonymous>
2018-10-03T05:41:41.674029925Z     at process._tickCallback (internal/process/next_tick.js:182:7)
2018-10-03T05:41:41.674033175Z Processing log blockNumber=6441289 transactionIndex=33

The reference to "toLowerCase" makes me guess that it is related to a place in the code where we lower case addresses before comparing them. And the address is undefined, maybe because the transaction was finalized and as such the Offer data deleted in the blockchain ?

Need some investigation...

franckc commented 5 years ago

@DanielVF @sparrowDom FYI in case you have an idea. This is not blocking for the release...

franckc commented 5 years ago

Checked and https://dapp.originprotocol.com/#/listing/1-000-10 is in "pending" state. Meaning a buyer made an offer but the sale was not finalized yet. So my hunch about Offer data deleted from blockchain is probably not correct.

cuongdo commented 5 years ago

Using Remix on mainnet, I can confirm that the single offer for listing 10 is not deleted:

image

The offer status is 1, so it's in the "created" state.

DanielVF commented 5 years ago

One easy way to get this error is to have an offer without an arbitrator or affiliate set, or to not have the arbitrator or affiliate set for the listener.

See code here:

https://github.com/OriginProtocol/origin-js/blob/714ce4d5e7d2e3b051abedc0f23a5b783bdfc312/src/resources/marketplace.js#L136-L143

Given that the offer appears to have this data, do we know if the listener has the arbitrator and affiliate set?

DanielVF commented 5 years ago

I'm going to dig deeper on this one - the listener is definitely not picking up that offer, while seeming to pick up other offers.

franckc commented 5 years ago

I took a look and was able to reproduce in my local environment, using this command: node listener/listener.js --web3-url=https://mainnet.infura.io/emIXjs9eDuy57IlTYsIP --ipfs-url=https://ipfs.originprotocol.com --continue-file=continue My continue file was set to: { "lastLogBlock": 6440925 }

Daniel's hunch was correct... The issue is with an offer that does not have an arbitrator set. I assume it happened on early mainnet listings when the arbitrator account wasn't set in configs...

I wish we would throw validation specific exception in the marketplace code vs the generic "throw new Error". Currently there is no way to differentiate between the types of errors to decide if worth retrying or if we should skip indexing the data... I'm leaning towards updating the listener to have the following behavior:

franckc commented 5 years ago

Fixed in https://github.com/OriginProtocol/origin/pull/737