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

ERC1155 for Spaceships and Artifacts #10

Merged
merged 53 commits into from
Oct 5, 2022

Conversation

cha0sg0d
Copy link

@cha0sg0d cha0sg0d commented Sep 20, 2022

Closes DFD-241 and Closes DFD-238

Most important point: tokenId refers to a collection of tokens of the same type, not a unique identifier for a single token. For example, in DF, tokenId 1 could refer to all the wormholes minted in the game.

There is no longer a concept of a unique token Id. There is just unique collection ids with 1 or more of the same tokens. So spaceships or artifacts -- which sound like a collection but are actually unique tokens -- would each have their own token Id.

References:

EIP
Example -> Contract Sample section
Test of base behavior

General Questions:

  • Where are tokenURIs set? -> ERC1155 Metadata

  • Do we need approval for anything? (allow third party to manage tokens)

  • Does the Diamond also need to be ERC1155-Receiver?

  • How does metadata work?

  • The metadata associated with them would then look like the following: https://my-uri/{whaleId}.json. Each collection has its own uri. tokenURI[tokenId] = https://token-cdn-domain/{id}.json. This makes sense for flexibility, but it means that we have to add some extra getters to have an O(1) lookup to see what type of token it is. For example, isSpaceship(tokenId) could internally have a mapping of (uint256 => bool) and returns true for any tokenId (collection) that refers to a spaceship. If Titans are id 1 and Motherships are id 2, isSpaceship would return true for input 1 or 2. However, this mapping needs to be manually created for faster lookups, because according to ERC1155, Titans and Motherships are completely unique tokens.

Maybe I'm wrong, but for CryptoPunks using ERC1155, you'd need to make a unique collection for each individual punk?

This Reddit answer agrees: It's really hard to handle multiple instances of unique tokens (like players owning different Wormholes). You can't look up ownerOf(tokenId) or interact with a specific instance of a game item.

Usage in Dark Forest Questions:

  • Define collection Ids for Spaceships, Artifacts, and Trophies (name tbd).

  • Figure out what type of mint (safeMint or normal Mint) is better.

  • SafeMint: revert if applicable transfer recipient is not valid ERC1155Receiver. Seems like a good practice, but not necessary for v1.

  • Figure out where to use batch Transfer or batch mint. (Could be useful in artifact store).

  • How do check if a specific instance of a token in a collection exists? ie, how do I get Wormhole 1 owned by Velorum vs Wormhole 2 owned by Ner0nzz?

First Step:

  1. Just replace existing functionality with ERC1155 calls.

@cha0sg0d
Copy link
Author

cha0sg0d commented Sep 24, 2022

Starting a new comment for a separate implementation discussion here.

References:
Game Dev having problems with ERC1155 vs ERC721
EIP discussion about NFTs

The problem: The ERC1155 Standard does not provide a native way to have a collection of unique NFTs.

Let's examine Wormholes in Dark Forest to illuminate this problem. A Wormhole is type of token that has certain properties (shrink planet distance). Lets say it has tokenId = 1. In the course of a match, there are multiple unique (non-fungible) Wormholes minted. Ner0nzz finds a Wormhole and so does Velorum. These two Wormholes share a type, but are wholly separate items. Each Wormhole has an owner and can be transferred or burned.

When Velorum wants to activate his wormhole, a problem occurs in the basic ERC1155 implementation:
How do you get the id of Velorum's wormhole? The token contract only knows the quantity of tokens with tokenId = 1. It can find that quantity by calling balanceOf(Velorum,1). If we bin by collection, the main benefit of ERC1155, we lose the ability to do basic operations in the game that require accessing a specific token.

The question: How should we implement collections of unique tokens?

There are three methods I have found:

  1. Keep the code as is. New token types, such as a Trophy, get a new ArtifactType which is functionally the unique id for identifying a collection of tokens. Whenever the game wants to do something with a Trophy token, it has look up the type of token associated with the id.

ex:

    // movedArtifactId can be a Spaceship or an Artifact
    function _isSpaceshipMove(DFPMoveArgs memory args) private view returns (bool) {
        return LibArtifactUtils.isSpaceship(gs().artifacts[args.movedArtifactId].artifactType);
    }

Pros:

Cons:

  • No way to add new fungible or semi-fungible token types.
  • Ugly to hardcode all of the logic for how to handle various artifact types.
  1. Use the Split-ID method outlined here.

We turn each uint256 tokenId in ERC1155 (refers to a collection) into <uint128: base token id><uint128: index of non-fungible>.

ex: In the Wormhole example, we'd get <uint128 wormhole type id><wormhole item id>. Ner0nzz: <uint128 1><uint128 1>. Velorum <uint128 1><uint128 2>. This would also require rewriting all of the functions like mint and burn to correctly account for this update in logic.

Pros:

  • We get collections of unique tokens inside ERC1155
  • We can add real NFTs with supply 1 or fungibles with higher supply.
  • Silver can be an ERC1155 but it's not ERC20 so no problems with ERC20s.

Cons:

  • Extra implementation work and testing. Reference implementations exist but seem highly specialized and moving away from the industry standard.
  1. Use the Natural method (

Another simple way to represent non-fungibles is to allow a maximum value of 1 for each non-fungible token. This would naturally mirror the real world, where unique items have a quantity of 1 and fungible items have a quantity greater than 1.

In this implementation, we would simply generate a random id for each token minted and make it its own collection. This is similar to the existing ERC721 implementation. We would store a typeId property on the new token

Pros:

  • Reflects true non-fungible nature of spaceships and artifacts.
  • Future benefits of ERC1155.

Cons:

  • Harder to conceptually bin similar tokens together.
  • Still need to create manual lookups to get token type based on token Id.

@cha0sg0d
Copy link
Author

cha0sg0d commented Sep 26, 2022

Artifact functions we need to think about:
create (mint)
activate / deactivate.

  • Move / remove artifactId to activeArtifact

burn (burn)

  • deposit

increment contract's balance, decrement contract's balance, add artifactId to planetArtifacts

withdraw

  • increment player's balance, decrement contract's balance

  • transfer

@cha0sg0d
Copy link
Author

cha0sg0d commented Sep 27, 2022

How we get an artifact?

  • We look up its id and owner.

  • We decode id into stats.

  • Does Biome change if an artifact moves? (no)

  • Make sure spaceships get put on planets.

@cha0sg0d
Copy link
Author

cha0sg0d commented Sep 27, 2022

Going down Route No. 4, suggested by Phated:

  • Artifacts and Spaceships are semi-fungible NFTs. They are either owned by the core contract, or a player.

  • A Spaceship cannot be transferred (TODO).

  • Photoid Cannons and Wormholes are gone (Temporarily).

  • Cooldown is gone.

  • You can either activate an artifact or deactivate it immediately.

  • Artifacts can either be reused infinitely or are burned on activation.

  • A Spaceship effect is applied on arrival.

  • An Artifact effect is applied on activation

Voyages?

  • When an artifact is on a voyage, it doesn't exist on a planet.

@cha0sg0d
Copy link
Author

cha0sg0d commented Sep 28, 2022

TODO:

  • Make Crescent activation work (or get rid of it).
  • Test Planetary Shield Functionality
  • Clean up testing logic (getting artifact, confirming burning, etc..)
  • Clean up console logs in contracts.
  • Make sure createArtifact logic is correct now that artifacts are unique.
  • Swap out existing bulk artifact getters with ERC1155Enumerable functions. See if useful.
  • client
  • Figure out gameUtils vs artifactUtils.

Notes:

  • Cooldown is gone. It's possible to add it back, but just seems confusing.

@phated you can check out Tokens for a full description of what I did.

Not sure if my _beforeTokenTransferHook broke ERC1155 Enumerable.

    /**
     * @notice ERC1155 hook: update aggregate values
     * @inheritdoc ERC1155BaseInternal
     */
     ```

Copy link
Collaborator

@phated phated left a comment

Choose a reason for hiding this comment

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

This PR is 🔥 - I love love love this work! I had some feedback. Note that I didn't review DFArtifacts.test.ts or DFSpaceships.test.ts because there's a TON 💯


Each token in Dark Forest is a collection under the ERC1155 Standard.

Each collection has a `tokenId` can be fungible (supply > 1) or non-fungible (supply = 1).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Each collection has a `tokenId` can be fungible (supply > 1) or non-fungible (supply = 1).
Each collection has a `tokenId` and can be fungible (supply > 1) or non-fungible (supply = 1).

eth/contracts/Tokens.md Outdated Show resolved Hide resolved
eth/contracts/Tokens.md Outdated Show resolved Hide resolved
eth/contracts/Tokens.md Outdated Show resolved Hide resolved
eth/contracts/Tokens.md Outdated Show resolved Hide resolved
eth/tasks/deploy.ts Outdated Show resolved Hide resolved
eth/contracts/DFTypes.sol Outdated Show resolved Hide resolved
eth/contracts/DFTypes.sol Outdated Show resolved Hide resolved
eth/contracts/facets/DFAdminFacet.sol Outdated Show resolved Hide resolved
eth/package.json Outdated
@@ -20,6 +20,7 @@
"@projectsophon/workspace": "^2.0.0",
"@solidstate/contracts": "^0.0.41",
"@solidstate/hardhat-4byte-uploader": "^1.0.2",
"@solidstate/spec": "^0.0.41",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you actually using this somewhere to implement in the tests?

Copy link
Author

Choose a reason for hiding this comment

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

Not anymore, I'll remove it.


address owner;
// function getArtifactById(uint256 artifactId)
Copy link
Author

Choose a reason for hiding this comment

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

We can just delete this, but might want to add it back bc of #27

eth/package.json Outdated Show resolved Hide resolved
cha0sg0d and others added 16 commits October 3, 2022 06:14
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
* Store artifact information on each planet

* Remove artifact task, as it does not make sense anymore

* Comment out debug tasks until we have token APIs settled

* Add ID logging to the prettyPrintToken util

* fix: add extra timeout

Co-authored-by: cha0sg0d <0xcha0sg0d@gmail.com>
* contracts: Separate carried spaceship from carried artifact

* contracts: Actually call the beforeTokenTransfer super call
* feat: working DFTokenFacet

* add phated changes + new facet

* feat: get players aritfacts and ships + tests. and refactor lol

* remove unused modifier

* remove silver token

* fix: spaceship found event

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
* client artifacts hacked to pieces

* client: Implement ArtifactInfo and fix artifact decoding

* client: Implement spaceship types and serde

* client: Re-enable spaceships in arrival utils

* client: Removing artifact map stuff

* client: Re-enable hasGear check

* client: Split spaceships from Artifacts in renderer

* client: Render spaceships around planet

* client: Add spaceship filename util

* client: Split artifactImage and spaceshipImage

* client: Fix mine artifact button without any ships

* client: Remove some comment spaceship code

* client: Remove canActivateArtifact because cooldowns were deleted

* chore: Format code

* client: Implement fetching wallet artifacts

* client: Implement an initial spaceship download

* client: Cache the artifacts & spaceships in player wallet

* get the stupid fucking code out of the types package

* client: Remove unused and unneeded function

* client: cleanup share

* client: Working to remove isSpaceShip throughout

* client: Fix spaceship decoder

* client: tokenType cleanup

* client: More light cleanup

* client: Split ArtifactsList & SpaceshipsList

* client: Remove empty message from ArtifactsList

* client: Allow sending spaceships via a SpaceshipRowg

* client: Fix sending a spaceship

* client: Cleanup ArtifactRow

* client: Remove location from headers

* client: Death to isSpaceShip

* client: Fix some artifactId conversion logic

* client: Begin getting ArtifactDetails working again

* client: Fix wormhole rendering

* client: Re-enable ArtifactDetailsPane & fetch artifact upgrades

* client: Stop lying

* client: Spaceship details & hover

* client: Force vite to rebuild deps

* client: Fix another ethers decoding problem

* client: Remove spaceship code from ArtifactDetailsPane

* client: More cleanup in ArtifactDetailsPane

* client: Sync artifact inventory & fix some artifact action state

* client: Put artifact & spaceship images in the PlanetDex

* client: Note the shitty hover component

* Update packages/procedural/src/ArtifactProcgen.ts

* chore: Format code

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@0xhank 0xhank marked this pull request as ready for review October 5, 2022 09:03
@0xhank 0xhank merged commit d492a36 into main Oct 5, 2022
@0xhank 0xhank deleted the cha0s/dfd-216-use-erc1155-standard-for-spaceships-and branch October 5, 2022 09:03
@cha0sg0d cha0sg0d changed the title Cha0s/dfd-216-use-erc1155-standard-for-spaceships-and ERC1155 for Spaceships and Artifacts Oct 5, 2022
cha0sg0d added a commit that referenced this pull request Oct 5, 2022
* test file

* feat: load SolidStateERC1155

* temp: noodling around

* test: working mint and uri tests

* chore: comment

* feat: sample bit pack

* feat: not working artifact refactor

* temp: working artifact activate and deactivate

* test: some test starting to work:

* feat: all artifact tests passing (no photoid or planetary shield)

* move tests work almost but need to totally redo spaceships

* feat: spaceships work

* feat: all tests pass + photoid. Need crescent and planetary shield then clean up then clinet

* failing test :(

* feat:  crescent test

* feat: shield test passes

* fix: clean up tests a bit

* feat: prevent spaceship transfer

* feat: bye bye DFToken

* feat: Readme

* remove more data structures

* feat: give artifacts unique ids for getter purposes

* feat: encode / decode works, need to thread through

* rename collection type

* feat: tests pass with spaceships as separate entities

* spaceship type

* fix: photoid test

* update types and readme

* chore: move functions that can be internal into LibArtifact and LibSpaceship

* fix: return when found in getter

* README tweaks

* feat: explain chunk properties in README

* fix: ordering of args for artifact events

* fix: split up getActiveArtifact into two checks

* fix: remove @solidstate/spec

* feat: create for Spaceship and Artifact

* feat: simply business logic bc of asserts in token create

* Store artifact information on contract planets (#28)

* Store artifact information on each planet

* Remove artifact task, as it does not make sense anymore

* Comment out debug tasks until we have token APIs settled

* Add ID logging to the prettyPrintToken util

* fix: add extra timeout

* More 1155 changes (#33)

* contracts: Separate carried spaceship from carried artifact

* contracts: Actually call the beforeTokenTransfer super call

* Cha0s/erc1155 ctd (#34)

* feat: working DFTokenFacet

* add phated changes + new facet

* feat: get players aritfacts and ships + tests. and refactor lol

* remove unused modifier

* remove silver token

* fix: spaceship found event

* fix: bye bye spaceship zero moves (#35)

* feat: ERC1155 clientside changes (#27)

* client artifacts hacked to pieces

* client: Implement ArtifactInfo and fix artifact decoding

* client: Implement spaceship types and serde

* client: Re-enable spaceships in arrival utils

* client: Removing artifact map stuff

* client: Re-enable hasGear check

* client: Split spaceships from Artifacts in renderer

* client: Render spaceships around planet

* client: Add spaceship filename util

* client: Split artifactImage and spaceshipImage

* client: Fix mine artifact button without any ships

* client: Remove some comment spaceship code

* client: Remove canActivateArtifact because cooldowns were deleted

* chore: Format code

* client: Implement fetching wallet artifacts

* client: Implement an initial spaceship download

* client: Cache the artifacts & spaceships in player wallet

* get the stupid fucking code out of the types package

* client: Remove unused and unneeded function

* client: cleanup share

* client: Working to remove isSpaceShip throughout

* client: Fix spaceship decoder

* client: tokenType cleanup

* client: More light cleanup

* client: Split ArtifactsList & SpaceshipsList

* client: Remove empty message from ArtifactsList

* client: Allow sending spaceships via a SpaceshipRowg

* client: Fix sending a spaceship

* client: Cleanup ArtifactRow

* client: Remove location from headers

* client: Death to isSpaceShip

* client: Fix some artifactId conversion logic

* client: Begin getting ArtifactDetails working again

* client: Fix wormhole rendering

* client: Re-enable ArtifactDetailsPane & fetch artifact upgrades

* client: Stop lying

* client: Spaceship details & hover

* client: Force vite to rebuild deps

* client: Fix another ethers decoding problem

* client: Remove spaceship code from ArtifactDetailsPane

* client: More cleanup in ArtifactDetailsPane

* client: Sync artifact inventory & fix some artifact action state

* client: Put artifact & spaceship images in the PlanetDex

* client: Note the shitty hover component

* Update packages/procedural/src/ArtifactProcgen.ts

* chore: Format code

* fix: listen for SpaceshipFound event

* contracts: fix tests

* contracts: fix subgraph codegen and remove console.log

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
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.

3 participants