shawntabrizi / substrate-collectables-workshop

A guided tutorial for building an NFT marketplace with the Polkadot SDK
https://www.shawntabrizi.com/substrate-collectables-workshop/
MIT License
236 stars 101 forks source link

Code seems outdated #153

Closed domob1812 closed 4 years ago

domob1812 commented 4 years ago

First of all: Is there some complete, "ready to compile and test" version of the substratekitties final code (e.g. as a full pallet including the Cargo.toml file)?

I checked out a fresh substrate-node-template as well as the collectables workshop. Then I just copied over the 5/assets/5.3-finished-code.rs file into the node template at runtime/src/substratekitties.rs and added mod substratekitties; to the runtime's src/lib.rs file. As far as I understand, this should make cargo just compile the substrate kitties code without adding it into the runtime itself.

But then cargo build --release fails for me with a lot of errors. Some of them are related to differences in how the dependencies are named (e.g. frame_support vs support, sp_runtime vs runtime_primitives), and could be fixed with a proper Cargo.toml file. But some other errors seem to be genuine issues with the workshop code, e.g. support::dispatch::Result is now called DispatchResult.

Am I missing something here, or is the code outdated? Especially if you are (like me) new to Rust and Substrate, it can be quite confusing when example code produces errors, since then I'm never sure whether it is an issue with me or actually the tutorial / example.

JoshOrndorff commented 4 years ago

Your process of copying the file and adding mod substratekitties is correct and does what you expect.

This workshop is targets Substrate v1.0 which is indeed getting pretty old. The workshop itself does work if followed from start to finish as the instructions tell users to checkout the v1.0 branch to get started.

Because this tutorial targets an older version of Substrate, we are in the process of moving it to the "Other Tutorials" section of devhub. @shawntabrizi can probably say more about his intentions for updating it.

If you are interested in learning Substrate on the latest (almost v2.0) code, I'd recommend starting with the Proof of Existence Tutorial. Happy to provide more support as you need it :)

domob1812 commented 4 years ago

Ok cool, thanks for the clarifications (and confirming it wasn't just a complete misunderstanding on my side)! I've already gone through the PoE tutorial, and also have my own basic project set up. I was just looking for some specific details (in this particular case, how to add initial balances in my test fixture), which I tried to get from the kitties tutorial.

But if you are already aware that the tutorial is somewhat outdated and working on moving it, then I guess we can close this issue.

JoshOrndorff commented 4 years ago

I was just looking for some specific details (in this particular case, how to add initial balances in my test fixture)

Maybe the recipe's writeup on this topic will be useful for you? https://substrate.dev/recipes/testing/mock.html#new_test_ext-a-name--newexta

domob1812 commented 4 years ago

Thanks for the pointer - I actually tried that before, and got the error:

error[E0422]: cannot find struct, variant or union type `GenesisConfig` in crate `balances`
   --> src/tests.rs:115:15
    |
115 |     balances::GenesisConfig::<Test> {
    |               ^^^^^^^^^^^^^ not found in `balances`
    |
help: possible candidate is found in another module, you can import it into scope
    |
3   | use system::GenesisConfig;
    |

That's why I then looked around and tried to use the (slightly different) code from the kitties tutorial. :) But I've now settled for just explicitly adding in the data I need in each test case, which makes them easier to read anyway and is not too bad (in my case).