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

feat(examples): openocean #2678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Molaryy
Copy link
Contributor

@Molaryy Molaryy commented Aug 12, 2024

This PR integrates the Openocean gno files into the main gnoland repository.
I created this project alongside Alex to learn more about nfts for our program student.

You can find a link for the dapp here: https://gnopenocean.netlify.app/ , we still have a few fix to do when a user needs to switch network but overall it works pretty fine.

Packages added:

  • collection.gno (all the collections handlers and functions)
  • nft.gno (basic nft functions)

Realms added:

  • openocean.gno (basically where all the functions required for a marketplace are)

Relevant packages used:

  • avl
  • grc721
  • errors
  • ufmt

@Molaryy Molaryy requested review from a team as code owners August 12, 2024 00:50
@Molaryy Molaryy requested review from mvertes and ltzmaxwell and removed request for a team August 12, 2024 00:50
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Aug 12, 2024
@Molaryy Molaryy changed the title feat: openocean feat(examples): openocean Aug 12, 2024
Copy link

codecov bot commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.15%. Comparing base (0ef28e8) to head (bc610fa).
Report is 89 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2678      +/-   ##
==========================================
+ Coverage   60.13%   60.15%   +0.02%     
==========================================
  Files         560      561       +1     
  Lines       74911    74999      +88     
==========================================
+ Hits        45046    45117      +71     
- Misses      26490    26504      +14     
- Partials     3375     3378       +3     
Flag Coverage Δ
contribs/gnodev 61.40% <ø> (ø)
contribs/gnofaucet 14.46% <ø> (-0.86%) ⬇️
gno.land 64.75% <ø> (+0.56%) ⬆️
gnovm 64.13% <ø> (ø)
misc/genstd 80.54% <ø> (ø)
misc/logos 20.23% <ø> (ø)
tm2 62.05% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Kouteki Kouteki added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Oct 3, 2024
@thehowl thehowl requested a review from n0izn0iz October 31, 2024 09:51
@thehowl
Copy link
Member

thehowl commented Oct 31, 2024

@moul @zivkovicmilos how do you think we should proceed with this?

Copy link
Contributor

@n0izn0iz n0izn0iz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This architecture is not a good nft marketplace example IMO:

  • The marketplace (openocean realm) should operate on the grc721 interface and not a specific collection/nft implemementation
  • It should only expose marketplace related calls (Sell/Buy/Offer/etc..) and not the underlying grc721 methods
  • The marketplace should use some kind of grc721 registry to access collections (see grc20reg for a registry example)
  • The marketplace should keep the nfts on sale in escrow

logo string,
avaiableTokens uint64,
) {
caller := std.GetOrigCaller()
Copy link
Contributor

@n0izn0iz n0izn0iz Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is insecure, use std.PrevRealm().Addr() instead of std.GetOrigCaller()
see https://docs.gno.land/concepts/effective-gno#contract-level-access-control

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages. review/triage-pending PRs opened by external contributors that are waiting for the 1st review
Projects
Status: In Progress
Status: In Review
Development

Successfully merging this pull request may close these issues.

4 participants