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

fix barge connection #1729

Merged
merged 50 commits into from
Jul 17, 2023
Merged

fix barge connection #1729

merged 50 commits into from
Jul 17, 2023

Conversation

EnzoVezzaro
Copy link
Contributor

@EnzoVezzaro EnzoVezzaro commented Oct 6, 2022

Changes proposed in this PR:

  • add development network
  • add barge to supported networks
  • update get local config method
  • use barge compiled artifacts in the market
  • fix config for barge in ocean.js (done in Fix barge config ocean.js#1652)
  • check publish, edit, profile, etc...
  • update market readme
  • fixed balance check
  • fixed FRE consume

@vercel
Copy link

vercel bot commented Oct 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
catalunya ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 2:31pm
market ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 2:31pm
odc ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 2:31pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
dubai-challenge ⬜️ Ignored (Inspect) Jul 17, 2023 2:31pm

@netlify
Copy link

netlify bot commented Oct 6, 2022

Deploy Preview for market-oceanprotocol ready!

Name Link
🔨 Latest commit 41389f8
🔍 Latest deploy log https://app.netlify.com/sites/market-oceanprotocol/deploys/634426c0cbc3a5000894879e
😎 Deploy Preview https://deploy-preview-1729--market-oceanprotocol.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@EnzoVezzaro
Copy link
Contributor Author

EnzoVezzaro commented Oct 6, 2022

Here some update on how barge is interacting with the market:

We can run barge just by:

export AQUARIUS_VERSION=<version>
./start_ocean.sh

To connect barge to the market, we need:

  • change NEXT_PUBLIC_METADATACACHE_URI to local instance of barge http://localhost:5000
  • set NEXT_PUBLIC_MARKET_DEVELOPMENT to true
  • edit config in src/@utils/ocean.ts[L21] with values from local instance of barge (see development object in terminal)

To interact with barge you need to connect to the localhost in metamask (or other RPC you use)

If all it's correct you should see the development network enabled and you should be able to transact. To make sure everything works in publish, I had to write a patch for the provider url, as mac (seems that ubuntu doesn't have this issue) doesn't reconized the docker's internal network 172.15.0.4:8030. This is just temporary, as we are looking for ways to avoid it.

Screenshot 2022-10-06 at 09 02 11

the problem is that even though the tx goes throw successfully (on the market), on barge I see this error (at least that's the only error I'm able to find):

Screenshot 2022-10-06 at 08 07 42

This error is only visible when transacting on the development network. If you publish in another network, let's say mumbai, the tx is successful and I don't get any error in console, but the assets never gets indexed. I'd expect the other networks to work the same way production does. At the moment, I'm not able to see any asset although all txs were successful.

The goal is to make sure our configs are returned correctly from ocean.js (already fixing; everything should work under development network) and improve the way the market runs with barge.

@EnzoVezzaro EnzoVezzaro changed the title Fix/issue fix barge connection fix barge connection Oct 6, 2022
@kremalicious
Copy link
Contributor

kremalicious commented Oct 10, 2022

change NEXT_PUBLIC_METADATACACHE_URI to local instance of barge http://localhost:5000
set NEXT_PUBLIC_MARKET_DEVELOPMENT to true

This seems redundant, most likely using NEXT_PUBLIC_MARKET_DEVELOPMENT is enough. If set we use local metadata cache URI. But we already get all those local URLs from ocean.js config helper, so we don't need to redefine them again here, no? https://github.com/oceanprotocol/ocean.js/blob/main/src/config/ConfigHelper.ts

@EnzoVezzaro
Copy link
Contributor Author

EnzoVezzaro commented Oct 10, 2022

change NEXT_PUBLIC_METADATACACHE_URI to local instance of barge http://localhost:5000
set NEXT_PUBLIC_MARKET_DEVELOPMENT to true

This seems redundant, most likely using NEXT_PUBLIC_MARKET_DEVELOPMENT is enough. If set we use local metadata cache URI. But we already get all those local URLs from ocean.js config helper, so we don't need to redefine them again here, no? https://github.com/oceanprotocol/ocean.js/blob/main/src/config/ConfigHelper.ts

Yes, that's how it should work. Oceanjs should return the configs; these changes are just temporary, we need them to help us debug barge, but probably this new code will change soon. But you're right, we should get the metadata uri from oceanjs for 'development' too (it's already like that, but it just doesn't seem to work on mac. @bogdanfazakas can tell you more).

@kremalicious kremalicious mentioned this pull request Oct 10, 2022
@EnzoVezzaro
Copy link
Contributor Author

EnzoVezzaro commented Oct 10, 2022

FYI:

Configs compared
Screenshot 2022-10-10 at 06 24 05
tried also changing subgraph to local port 9000. It's always returns empty array (no results).

Logs publishing from barge
log.txt

@jamiehewitt15
Copy link
Member

When testing this at the moment. The "submit URL" button seems to be permanently inactive. Not seeing any particular errors in the console related to it
image

@jamiehewitt15
Copy link
Member

I'm still having the problem where it's not possible to submit the asset URL. Also, it looks like the provider URL is missing in the form.

image

@codeclimate
Copy link

codeclimate bot commented Jul 17, 2023

Code Climate has analyzed commit 6d83147 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4

The test coverage on the diff in this pull request is 27.7% (50% is the threshold).

This pull request will bring the total coverage in the repository to 20.3% (0.0% change).

View more on Code Climate.

@bogdanfazakas bogdanfazakas merged commit 99090ee into main Jul 17, 2023
13 checks passed
@bogdanfazakas bogdanfazakas deleted the fix/issue-fix-barge-connection branch July 17, 2023 20:42
@rogargon
Copy link

Just tried the features added by this branch merged into main but I was unable to make the market work with Barge.

I started barge as instructed in the README. I suppose I also need to copy the addresses of the deployed contracts using copy-contracts.sh. In any case, I run npm run set-barge-env after creating a sample .env file (otherwise it fails) and I get the following added:

# Development Preference Center
#NEXT_PUBLIC_NFT_FACTORY_ADDRESS='0xxx'
#NEXT_PUBLIC_OPF_COMMUNITY_FEE_COLECTOR='0xxx'
#NEXT_PUBLIC_FIXED_RATE_EXCHANGE_ADDRESS='0xxx'
#NEXT_PUBLIC_DISPENSER_ADDRESS='0xxx'
#NEXT_PUBLIC_OCEAN_TOKEN_ADDRESS='0xxx'
#NEXT_PUBLIC_MARKET_DEVELOPMENT='true'
#NEXT_PUBLIC_PROVIDER_URL="http://xxx:xxx"
#NEXT_PUBLIC_SUBGRAPH_URI="http://xxx:xxx"
#NEXT_PUBLIC_METADATACACHE_URI="http://127.0.0.1:5000" # only for mac
#NEXT_PUBLIC_RPC_URI="http://xxx:xxx"
NEXT_PUBLIC_OPF_COMMUNITY_FEE_COLECTOR=0x2fC1fd21cb222Dc180Ef817dE4c426fd9230b5A50
#NEXT_PUBLIC_SUBGRAPH_URI',"http://127.0.0.1:9000" # only for mac=undefined1F03e80_ADDRESS=0xe749E2f8482810b11B838Ae8c5eb69e54d610411dF01F03e80

I cannot get past this. Am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Market connected to Barge is not working properly
5 participants