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

Add Bubblegum and Candy-wrapper #671

Merged
merged 183 commits into from
Aug 12, 2022
Merged

Add Bubblegum and Candy-wrapper #671

merged 183 commits into from
Aug 12, 2022

Conversation

danenbm
Copy link
Contributor

@danenbm danenbm commented Aug 10, 2022

Update

  • Met with @thlorenz and got further advice on js files:
    • Moved bubblegum js dependencies to bubblegum specific package.json.
    • Removed extra tsconfig file and added typedoc file.
    • Moved and renamed main index.ts to src directory
  • Reverted some changes to Anchor.toml that were added to make anchor test run bubblegum-test.ts.
    • The reason for this is long term we aren't planning to run anchor test in this repo.
    • As a reference I left a version where anchor-test --skip-lint runs bubblegum-test.ts (but fails) at: danenbm/bubblegum-anchor-test-runs-fails
    • On that branch there are instructions to get the test to run: TEST_INSTRUCTIONS.md
  • Excluding candy-wrapper from github workflow.
  • NOTE some of the scripts in bubblegum/js/package.json do not work. This is because they fail due to issues with the tests, which are going to be reworked anyways. And at this time we do not need things like yarn check:publish-ready, etc. since we aren't publishing this yet.
  • api:gen is working which is what is needed now for CI tests.

Overall summary

  • This PR ports Bubblegum and Candy-wrapper from the Candyland repo.
  • In order to do that a few directories are moved around, and various package.json etc. files are updated to get the dependencies right.

Testing

  • anchor build --skip-lint works and builds bubblegum.so and candy-wrapper.so.
  • yarn api:gen works from bubblegum/js directory and builds
    same as existing IDL from Candyland repo (except adds a few metadata fields).

Previous PR

Detailed Notes

@danenbm

  • Code was merged from https://github.com/jarry-xiao/candyland repo.
  • Moved program from programs/bubblegum/ to bubblegum/program.
  • Moved sdk from contracts/sdk/bubblegum/ to bubblegum/js.
  • Moved tests from contracts/tests/ to bubblegum/js/test.
  • Moved program from programs/candy-wrapper to candy-wrapper/program.
  • Adding api:gen script to bubblegum js directory
  • Adding couple misc files from original repo.
  • Adding solita config.
  • Cleaning up some yarn.lock and package.json files.
  • Including Bubblegum in some top level config files.

@jshiohaha

  • enable bubblegum anchor tests
    • update package.json and tsconfig.json
    • cleanup bubblegum test file and imports
    • enable anchor test --skip-lint to run after anchor workspace members are built

@danenbm

  • Moving bubblegum js dependencies to bubblegum specific package.json.
  • Removing extra tsconfig file and adding typedoc file.
  • Moved and renamed index.ts to src directory.
  • Removing Anchor workspace additions and nonworking yarn scripts.
  • Excluding candy-wrapper from github workflow.

ngundotra and others added 30 commits May 27, 2022 16:33
Separate datastructure from gummyroll smart contract
Separate datastructure from gummyroll smart contract
remove MC from cargo lock
- update package.json and tsconfig.json
- cleanup bubblegum test file and imports
- enable anchor test --skip-lint to run after anchor workspace members are built
@danenbm danenbm marked this pull request as ready for review August 10, 2022 23:10
@danenbm danenbm requested a review from a team as a code owner August 10, 2022 23:10
@danenbm danenbm force-pushed the danenbm/add-bubblegum-hist branch from 0552eb7 to 6c6410e Compare August 11, 2022 22:58
- Also adding license files to js directories.
- Also cleaning up extra bubblegum types file.
thlorenz
thlorenz previously approved these changes Aug 12, 2022
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

Aside from the nits you can fix LGTM

bubblegum/js/typedoc.json Outdated Show resolved Hide resolved
bubblegum/js/package.json Outdated Show resolved Hide resolved
thlorenz
thlorenz previously approved these changes Aug 12, 2022
jshiohaha
jshiohaha previously approved these changes Aug 12, 2022
Copy link
Contributor

@jshiohaha jshiohaha left a comment

Choose a reason for hiding this comment

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

Few nit + informational comments, but overall LGTM - nice work!

bubblegum/program/Cargo.toml Outdated Show resolved Hide resolved
candy-wrapper/program/Cargo.toml Outdated Show resolved Hide resolved
@@ -6,7 +6,8 @@ cluster = "localnet"
wallet = "~/.config/solana/id.json"

[programs.localnet]
mpl_auction_house="hausS13jsjafwWwGqZTUQRmWyvyxn9EQpqMwV1PBBmk"
auction_house="hausS13jsjafwWwGqZTUQRmWyvyxn9EQpqMwV1PBBmk"
Copy link
Contributor

Choose a reason for hiding this comment

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

Informational note on why this was changed: when trying to anchor test --skip-lint, I got errors related to not being able to find IDL for mpl_auction_house. I realized this is because the IDL generated and stored in the MPL root target/idl/ directory was called auction_house.json and had name field auction_house.

So tldr; it looks like anchor was expecting mpl_auction_house (based on this naming) but found auction_house. But, I guess this doesn't matter if we aren't using Anchor workspaces.

@danenbm danenbm dismissed stale reviews from jshiohaha and thlorenz via eb60fbe August 12, 2022 18:35
@austbot austbot merged commit 4c45b58 into master Aug 12, 2022
@ngundotra
Copy link
Collaborator

🔥 🔥 🔥

@stegaBOB stegaBOB deleted the danenbm/add-bubblegum-hist branch September 12, 2022 02:12
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.

7 participants