Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

History links not working for some assets #773

Closed
mihaisc opened this issue Aug 4, 2021 · 11 comments · Fixed by #794
Closed

History links not working for some assets #773

mihaisc opened this issue Aug 4, 2021 · 11 comments · Fixed by #794
Assignees
Labels
Type: Bug Something isn't working

Comments

@mihaisc
Copy link
Contributor

mihaisc commented Aug 4, 2021

Some metadata history links are not working , tx is undefined

examples: https://market.oceanprotocol.com/asset/did:op:b98B96B2DDe7e1237928400cFCD54327eF58fff4

https://market.oceanprotocol.com/asset/did:op:7FBEDfF92F39ae900393B17370E192D508458FDB

@mihaisc mihaisc added Type: Enhancement New feature or request Type: Bug Something isn't working and removed Type: Enhancement New feature or request labels Aug 4, 2021
@jamiehewitt15
Copy link
Member

This seems to be a multinetwork issue. If you are not on the same network as the asset it will have undefined in the URL. If you are on the same network it works.

Interestingly, it seems to work fine with no wallet connected.

@jamiehewitt15
Copy link
Member

The issue is that the graphQL query isn't working when you're on the wrong network.

const [result] = useQuery({
  query: getReceipts,
  variables: { address: ddo?.dataToken.toLowerCase() }
})
const { data } = result
console.log('Data:', data) /// Data: undefined

Still not sure why it works when a wallet isn't connected

@jamiehewitt15
Copy link
Member

I noticed that the DDO already contains the id of the creation transaction, so the easiest way to solve this is by using:

ddo.event.txid

@kremalicious
Copy link
Contributor

kremalicious commented Aug 11, 2021

Totally missed that ddo.event was added over in Aquarius which is not documented anywhere and also not clear why this is just the creation transaction and not all events. Regardless, if it's there and really the creation transaction then we can use it but as this is just the creation tx can't see how this would solve the link for update transactions

@jamiehewitt15
Copy link
Member

Actually, @alexcos20 pointed out that ddo.event.txid is the transaction Id of the metadata (publish/update) event rather than the DT creation event. So still need another solution to this...

I'm struggling to see why the GraphQL query works when you don't have a wallet connected but doesn't work when you have a wallet connected to the wrong network.

@kremalicious
Copy link
Contributor

kremalicious commented Aug 11, 2021

I think we have to ditch useQuery() from urql as this will always use the client initiated in UrqlProvider which is based on the networkId. Might be worth rethinking that provider to also deal with a DDO but this could get messy in terms of logic. For me, did:op:7FBEDfF92F39ae900393B17370E192D508458FDB also does not work without wallet.

What we have done for getting the asset pool data is to pass another OperationContext and initiate the subgraph connection from components. E.g. here: https://github.com/oceanprotocol/market/blob/main/src/components/organisms/AssetActions/Pool/index.tsx#L103-L116

So we could do the same for edit history, and looks like it would make sense replacing the auto-connection of UrqlProvider with some connect method in there which is then shared among all the components which need to query subgraph independent of wallet network. And then raises question why we have UrqlProvider base everything on user wallet network when if I'm not mistaken we pretty much never use that.

@kremalicious
Copy link
Contributor

kremalicious commented Aug 11, 2021

Same is true for Consume.tsx btw which also uses useQuery and I get a bunch of console errors from this on every asset details. So apart from edit history and consume component we do not use useQuery and its provider anywhere

@mihaisc
Copy link
Contributor Author

mihaisc commented Aug 11, 2021

The asset details error is old and unrelated. But shouldn't the subgraph url be based on ddo network ,not wallet network?

@jamiehewitt15
Copy link
Member

jamiehewitt15 commented Aug 11, 2021

@mihaisc Yeah, based on the DDO network would get it working again...

Currently, it's based on the network ID, with a default of mainnet:

const oceanConfig = getOceanConfig(networkId || 1)

@jamiehewitt15
Copy link
Member

For me, did:op:7FBEDfF92F39ae900393B17370E192D508458FDB also does not work without wallet.

Yeah that's correct. It's actually only mainnet assets that are working without a wallet connect as the default network ID is 1

@kremalicious
Copy link
Contributor

kremalicious commented Aug 11, 2021

But shouldn't the subgraph url be based on ddo network ,not wallet network?

yes totally, that UrqlProvider looks useless, we just missed refactoring more after we had the new subgraph mechanisms in multinetwork.

But as both remaining useQuery uses are within the asset details view simple fix might be as @jamiehewitt15 suggested making that provider work by default with ddo.chainId. Then we just need to move that provider down to asset details page below the AssetProvider and we would have easy DDO access. After that, the existing useQuery should start working again in all the possible cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants