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

Update all submodules, related fixes and mint automatization #16

Merged
merged 37 commits into from
Jul 29, 2022

Conversation

ryukzak
Copy link
Contributor

@ryukzak ryukzak commented Jun 22, 2022

Simplify NFT minting by the scripts. Details in updated README.md

Currently, scripts allow to mint new NFT for images, which can be founded in the marketplace sometime later. But it is impossible to buy due to: NFT not found on marketplace

Related PRs:

State:

  • Minting. Can't reproduce with vasil network. Two problems:
  • Buying. Updated CTL and seabug contract. Tested on testnet with vasil and should work.

@ryukzak ryukzak changed the title Draft: bump nftmarketplace version Draft: bump nft-marketplace version Jun 22, 2022
@rynoV
Copy link
Contributor

rynoV commented Jun 28, 2022

I needed to add logLevel: 'Trace', here to get it to work, and should we also update this line in seabug-contracts/index.d.ts to include the logLevel?

@ryukzak ryukzak changed the title Draft: bump nft-marketplace version Draft: Update CTL and other dependency, related fixes and mint automatization Jul 8, 2022
@ryukzak ryukzak force-pushed the aleksandr/bump-nftmarketplace branch from 45a6771 to b94bbf0 Compare July 8, 2022 10:03
README.md Outdated Show resolved Hide resolved
arion-compose.nix Show resolved Hide resolved
"--server-api" "usr:pwd"
"--ogmios-address" "ogmios" "--ogmios-port" "1337"
"--origin" "--use-latest"
"--block-filter" "{\"address\": \"addr_test1wr05mmuhd3nvyjan9u4a7c76gj756am40qg7vuz90vnkjzczfulda\"}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This address seems like it shouldn't be hardcoded, is it the marketplace address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is. The issue on this: #18

Copy link
Member

Choose a reason for hiding this comment

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

This should be probably some const defined earlier in this file, it's constant after launch, it's marketplace escrow address

@rynoV rynoV changed the title Draft: Update CTL and other dependency, related fixes and mint automatization Update all submodules, related fixes and mint automatization Jul 27, 2022
@rynoV
Copy link
Contributor

rynoV commented Jul 27, 2022

@rynoV
Copy link
Contributor

rynoV commented Jul 27, 2022

With dfaedae, from a clean repo:

@t4ccer
Copy link
Member

t4ccer commented Jul 28, 2022

re using development server: Well, it's development server, we should rather fix nginx setup

ogmios-datum-cache = (import ogmios-datum-cache/default.nix).packages.x86_64-linux.ogmios-datum-cache;
nft-marketplace-server = (import nft-marketplace-server/default.nix).packages.x86_64-linux."nft-marketplace-server:exe:nft-marketplace-server";
ogmios-datum-cache = (import ogmios-datum-cache/default.nix).packages.x86_64-linux."ogmios-datum-cache";
# FIXME: CTL version also pinned in seabug-contract. We need only one source of truth
Copy link
Member

Choose a reason for hiding this comment

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

Will be fixed when we'll move from git submodules to flakes, then you can just follow input

arion-compose.nix Outdated Show resolved Hide resolved
"--server-api" "usr:pwd"
"--ogmios-address" "ogmios" "--ogmios-port" "1337"
"--origin" "--use-latest"
"--block-filter" "{\"address\": \"addr_test1wr05mmuhd3nvyjan9u4a7c76gj756am40qg7vuz90vnkjzczfulda\"}"
Copy link
Member

Choose a reason for hiding this comment

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

This should be probably some const defined earlier in this file, it's constant after launch, it's marketplace escrow address

scripts/mint-nft.sh Outdated Show resolved Hide resolved
README.md Outdated
@@ -69,73 +69,85 @@ Ensure that Nami is set to Testnet, that you have some Test Ada, and that you've

### Optional: Mint your own NFTs

This process will be simplified in the future.
UPDATE: see the minting section in `seabug-contracts/README.md` instead. The following minting process is currently broken, see [\#22](https://github.com/mlabs-haskell/seabug/issues/22).
Copy link
Member

Choose a reason for hiding this comment

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

Keeping broken process in readme can be confusing even if it's marked as broken

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure if its the right move to drop the full minting util, I feel like probably not, but we can certainly do so from the readme, and add back later once fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

The new minting process partially relies on mint-nft.sh to upload an image to IPFS+our database and get its base36 CID, and some of the setup from this section of the readme is necessary for that to work. It's kind of hacky right now, but I'm not sure if it's worth trying to extract out the part of mint-nft.sh that we need.

Maybe we just remove mint-nft.sh for now, but keeping the parts that are needed? The file is in git anyways

Copy link
Contributor

Choose a reason for hiding this comment

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

See 43dad7d for what I meant. I can revert that commit if we want to keep the old version, but I feel like it'll just cause confusion

@rynoV
Copy link
Contributor

rynoV commented Jul 28, 2022

re using development server: Well, it's development server, we should rather fix nginx setup

I can spend some time today fixing the nginx setup, I'm sure there's probably some tutorials for "nginx with <our react routing lib>" cc @samuelWilliams99 @t4ccer

nginx-default.conf is the /etc/nginx/conf.d/default.conf file copied
from the nginx container, and I just added the `try_files` line
@rynoV rynoV requested a review from t4ccer July 28, 2022 16:36
@Synthetica9
Copy link

Currently merging this into #19, so that can be merged shortly after this is merged.

@rynoV rynoV merged commit ac97bf3 into main Jul 29, 2022
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.

6 participants