OriginProtocol / origin

Monorepo for our developer tools and decentralized marketplace application
https://www.originprotocol.com/developers
MIT License
652 stars 196 forks source link

DApp ignores network number in URLs #2045

Open wanderingstan opened 5 years ago

wanderingstan commented 5 years ago

A listings ID includes the network number to ensure uniqueness and avoid confusions.

However, dapp currently ignore that number. E.g. 4-000-522-4181140 is on network 4 (Rinkeby) but the main dapp will happily display 1-000-522-4181140 instead:

https://dapp.originprotocol.com/#/listing/4-000-522-4181140

https://dapp.staging.originprotocol.com/#/4-000-522-4181140

micahalcorn commented 5 years ago

It seems that the hostname determines which listing to display.

I get "DECENTRALIZED REALITY" at both of these URLs:

https://dapp.originprotocol.com/#/listing/1-000-522-4181140 https://dapp.originprotocol.com/#/listing/4-000-522-4181140

And I get "Listener Test" at both of these:

https://dapp.staging.originprotocol.com/#/listing/1-000-522-4181140 https://dapp.staging.originprotocol.com/#/listing/4-000-522-4181140

wanderingstan commented 5 years ago

We should put a check in GraphQL to compare the first digit of the id (network number) with its network setting. If they don't match, nothing should be returned. I.e. if ID begins with 4 (rinkeby), and network is set to mainnet, then GraphQL should not return anything.

image

wanderingstan commented 5 years ago

A little digging shows that the network ID is parsed out in the oft-used function parseId() here: https://github.com/OriginProtocol/origin/blob/master/packages/graphql/src/utils/parseId.js

It returns an "internal only" value (confusingly?) also called listingId, which refers to the number after the contract number. The 522 in 4-000-522. This is the value that is used internally within the graphql package.

However, the netId param returned (4) does't seem to get used, e.g. here: https://github.com/OriginProtocol/origin/blob/master/packages/graphql/src/resolvers/Listing.js#L13

Maybe we could put a check within the parseId function that throws and error or warning when the network id doesn't match what graphql is configured to use?