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

Standard v2 Cadence 1.0 updates #194

Merged
merged 31 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ee5c99e
Bump github.com/ethereum/go-ethereum in /lib/go/test
dependabot[bot] Sep 6, 2023
ca0353f
Bump golang.org/x/net from 0.10.0 to 0.17.0 in /lib/go/test
dependabot[bot] Oct 11, 2023
03914e9
remove js packages
joshuahannan Oct 19, 2023
21640aa
Merge branch 'master' into standard-v2-updates
sisyphusSmiling Oct 20, 2023
1a261c0
update NFTForwarding for Cadence 1.0
sisyphusSmiling Oct 20, 2023
d9ef40e
update NFTForwarding transactions for new implementation
sisyphusSmiling Oct 20, 2023
2d9a34c
bump ci flow cli version to Cadence 1.0 pre-release
sisyphusSmiling Oct 23, 2023
8014190
fix ExampleNFT-v2 account storage access bug
sisyphusSmiling Oct 24, 2023
4311a6c
update FungibleToken Cadence 1.0 implementation
sisyphusSmiling Oct 24, 2023
262c73f
update flow.json with v2 contract implementations
sisyphusSmiling Oct 24, 2023
d872c31
update txns & scripts for Cadence 1.0 + supporting ExampleNFT-v2 Cade…
sisyphusSmiling Oct 24, 2023
c6330af
update Makefile & go tests
sisyphusSmiling Oct 24, 2023
ef9b4d7
update NFT-v2 to emit events from interface pre/post conditions
sisyphusSmiling Oct 24, 2023
359da96
update ExampleNFT-v2 Cadence tests & go assets
sisyphusSmiling Oct 24, 2023
0649cdf
update ExampleNFT-v2 providerPath
sisyphusSmiling Oct 24, 2023
b959c5b
update go tests and supporting txns
sisyphusSmiling Oct 24, 2023
92526d4
update go assets
sisyphusSmiling Oct 24, 2023
42b00b3
fix test cases to correspond with new contract implementations
sisyphusSmiling Oct 24, 2023
0f2bb28
add metadata trait test validation to go suite
sisyphusSmiling Oct 24, 2023
6958b19
update flow.json with emulator aliases
sisyphusSmiling Oct 24, 2023
cf9a423
fix NFTForwarding.Forwarder conformance to .Receiver & rename events
sisyphusSmiling Oct 24, 2023
c7aa593
add NFTForwarding Cadence tests & fix txn bugs
sisyphusSmiling Oct 24, 2023
cb3b555
complete NFTForwarding cadence tests + supporting txns & scripts
sisyphusSmiling Oct 24, 2023
f42de59
update .gitignore to include coverage.json
sisyphusSmiling Oct 24, 2023
88a73f1
fix contract.go placeholder variable name
sisyphusSmiling Oct 26, 2023
3c50d77
update NFT.transfer() pre-conditions on recevier.check() & .getIDs().…
sisyphusSmiling Nov 6, 2023
60eb711
update instances of Collection.getIDs().length with .getLength()
sisyphusSmiling Nov 6, 2023
8531ac6
add comments to test_helpers methods
sisyphusSmiling Nov 7, 2023
4347b53
update go assets
sisyphusSmiling Nov 7, 2023
86ca92b
re-add .transfer() pre-condition on .getIDs().contains(id)
sisyphusSmiling Nov 7, 2023
3f3d5a7
update go assets
sisyphusSmiling Nov 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,12 @@ jobs:
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-
- uses: actions/setup-node@v3
with:
node-version: 16
cache: 'npm'
cache-dependency-path: lib/js/test/package-lock.json
- name: Install Flow CLI
run: sh -ci "$(curl -fsSL https://raw.githubusercontent.com/onflow/flow-cli/master/install.sh)" -- v1.3.1
run: sh -ci "$(curl -fsSL https://raw.githubusercontent.com/onflow/flow-cli/master/install.sh)" -- v1.5.0-stable-cadence.3
- name: Flow CLI Version
run: flow version
- name: Update PATH
run: echo "/root/.local/bin" >> $GITHUB_PATH
- name: Install dependencies
run: cd lib/js/test && npm ci
- name: Run tests
run: make ci

18 changes: 1 addition & 17 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,4 @@ jobs:
name: 🚀 release
runs-on: ubuntu-latest
steps:
- name: 📚 checkout
uses: actions/[email protected]
- name: 🟢 node
uses: actions/[email protected]
with:
node-version: 15
registry-url: https://registry.npmjs.org
- name: Install Dependencies
run: npm i
- name: Create Release Pull Request or Publish to npm
id: changesets
uses: changesets/action@v1
with:
publish: npm run release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
- name: 📚 checkout
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.idea
.vscode
node_modules
node_modules
coverage.json
6 changes: 2 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
test:
$(MAKE) generate -C lib/go
$(MAKE) test -C lib/go
$(MAKE) test -C lib/js/test
flow test --cover tests/test_example_nft.cdc
flow test --cover tests/*_tests.cdc

.PHONY: ci
ci:
$(MAKE) ci -C lib/go
$(MAKE) ci -C lib/js/test
flow test --cover tests/test_example_nft.cdc
flow test --cover tests/*_tests.cdc
29 changes: 17 additions & 12 deletions contracts/ExampleNFT-v2.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ access(all) contract ExampleNFT: MultipleNFT, ViewResolver {
return self.ownedNFTs.keys
}

/// Gets the amount of NFTs stored in the collection
access(all) view fun getLength(): Int {
sisyphusSmiling marked this conversation as resolved.
Show resolved Hide resolved
return self.ownedNFTs.keys.length
}

access(all) view fun getIDsWithTypes(): {Type: [UInt64]} {
let typeIDs: {Type: [UInt64]} = {}
typeIDs[Type<@ExampleNFT.NFT>()] = self.getIDs()
Expand All @@ -251,9 +256,10 @@ access(all) contract ExampleNFT: MultipleNFT, ViewResolver {

/// Borrow the view resolver for the specified NFT ID
access(all) view fun borrowViewResolver(id: UInt64): &{ViewResolver.Resolver}? {
let nft = (&self.ownedNFTs[id] as &ExampleNFT.NFT?)!
let exampleNFT = nft as! &ExampleNFT.NFT
return exampleNFT as &{ViewResolver.Resolver}
if let nft = &self.ownedNFTs[id] as &ExampleNFT.NFT? {
return nft as &{ViewResolver.Resolver}
}
return nil
}

/// public function that anyone can call to create a new empty collection
Expand Down Expand Up @@ -336,12 +342,13 @@ access(all) contract ExampleNFT: MultipleNFT, ViewResolver {
access(all) view fun getCollectionData(nftType: Type): MetadataViews.NFTCollectionData? {
switch nftType {
case Type<@ExampleNFT.NFT>():
let collectionRef = self.account.borrow<&ExampleNFT.Collection>(from: /storage/cadenceExampleNFTCollection)
?? panic("Could not borrow a reference to the stored collection")
let collectionRef = self.account.storage.borrow<&ExampleNFT.Collection>(
from: /storage/cadenceExampleNFTCollection
) ?? panic("Could not borrow a reference to the stored collection")
let collectionData = MetadataViews.NFTCollectionData(
storagePath: collectionRef.getDefaultStoragePath()!,
publicPath: collectionRef.getDefaultPublicPath()!,
providerPath: /private/exampleNFTCollection,
providerPath: /private/cadenceExampleNFTCollection,
publicCollection: Type<&ExampleNFT.Collection>(),
publicLinkedType: Type<&ExampleNFT.Collection>(),
providerLinkedType: Type<auth(NonFungibleToken.Withdrawable) &ExampleNFT.Collection>(),
Expand Down Expand Up @@ -424,17 +431,15 @@ access(all) contract ExampleNFT: MultipleNFT, ViewResolver {
let collection <- create Collection()
let defaultStoragePath = collection.getDefaultStoragePath()!
let defaultPublicPath = collection.getDefaultPublicPath()!
self.account.save(<-collection, to: defaultStoragePath)
self.account.storage.save(<-collection, to: defaultStoragePath)

// create a public capability for the collection
self.account.link<&ExampleNFT.Collection>(
defaultPublicPath,
target: defaultStoragePath
)
let collectionCap = self.account.capabilities.storage.issue<&ExampleNFT.Collection>(defaultStoragePath)
self.account.capabilities.publish(collectionCap, at: defaultPublicPath)

// Create a Minter resource and save it to storage
let minter <- create NFTMinter()
self.account.save(<-minter, to: self.MinterStoragePath)
self.account.storage.save(<-minter, to: self.MinterStoragePath)
}
}

43 changes: 21 additions & 22 deletions contracts/NonFungibleToken-v2.cdc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ access(all) contract NonFungibleToken {
///
access(all) event Withdraw(id: UInt64, uuid: UInt64, from: Address?, type: String)

access(self) fun emitNFTWithdraw(id: UInt64, uuid: UInt64, from: Address?, type: String): Bool
access(self) view fun emitNFTWithdraw(id: UInt64, uuid: UInt64, from: Address?, type: String): Bool
{
emit Withdraw(id: id, uuid: uuid, from: from, type: type)
return true
Expand All @@ -70,7 +70,7 @@ access(all) contract NonFungibleToken {
///
access(all) event Deposit(id: UInt64, uuid: UInt64, to: Address?, type: String)

access(self) fun emitNFTDeposit(id: UInt64, uuid: UInt64, to: Address?, type: String): Bool
access(self) view fun emitNFTDeposit(id: UInt64, uuid: UInt64, to: Address?, type: String): Bool
{
emit Deposit(id: id, uuid: uuid, to: to, type: type)
return true
Expand All @@ -82,7 +82,7 @@ access(all) contract NonFungibleToken {
///
access(all) event Transfer(id: UInt64, uuid: UInt64, from: Address?, to: Address?, type: String)

access(self) fun emitNFTTransfer(id: UInt64, uuid: UInt64?, from: Address?, to: Address?, type: String?): Bool
access(self) view fun emitNFTTransfer(id: UInt64, uuid: UInt64?, from: Address?, to: Address?, type: String?): Bool
{
// The transfer method can return false even if it didn't do a transfer
// in which case we don't want the event to be emitted
Expand All @@ -99,7 +99,7 @@ access(all) contract NonFungibleToken {
/// The event that should be emitted when an NFT is destroyed
access(all) event Destroy(id: UInt64, uuid: UInt64, type: String)

access(self) fun emitNFTDestroy(id: UInt64, uuid: UInt64, type: String): Bool
access(self) view fun emitNFTDestroy(id: UInt64, uuid: UInt64, type: String): Bool
{
emit Destroy(id: id, uuid: uuid, type: type)
return true
Expand All @@ -122,7 +122,7 @@ access(all) contract NonFungibleToken {

destroy() {
pre {
//NonFungibleToken.emitNFTDestroy(id: self.getID(), uuid: self.uuid, type: self.getType().identifier)
NonFungibleToken.emitNFTDestroy(id: self.getID(), uuid: self.uuid, type: self.getType().identifier)
}
}
}
Expand All @@ -143,7 +143,7 @@ access(all) contract NonFungibleToken {
access(Withdrawable) fun withdraw(withdrawID: UInt64): @{NFT} {
post {
result.getID() == withdrawID: "The ID of the withdrawn token must be the same as the requested ID"
//NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
}
}

Expand All @@ -161,7 +161,7 @@ access(all) contract NonFungibleToken {
access(Withdrawable) fun withdrawWithUUID(_ uuid: UInt64): @{NFT} {
post {
result == nil || result!.uuid == uuid: "The ID of the withdrawn token must be the same as the requested ID"
//NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
}
}

Expand All @@ -170,7 +170,7 @@ access(all) contract NonFungibleToken {
access(Withdrawable) fun withdrawWithType(type: Type, withdrawID: UInt64): @{NFT} {
post {
result == nil || result.getID() == withdrawID: "The ID of the withdrawn token must be the same as the requested ID"
//NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
}
}

Expand All @@ -179,7 +179,7 @@ access(all) contract NonFungibleToken {
access(Withdrawable) fun withdrawWithTypeAndUUID(type: Type, uuid: UInt64): @{NFT} {
post {
result == nil || result!.uuid == uuid: "The ID of the withdrawn token must be the same as the requested ID"
//NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
NonFungibleToken.emitNFTWithdraw(id: result.getID(), uuid: result.uuid, from: self.owner?.address, type: result.getType().identifier)
}
}
}
Expand Down Expand Up @@ -247,7 +247,7 @@ access(all) contract NonFungibleToken {
/// and returns it to the caller so that they can own NFTs
access(all) fun createEmptyCollection(): @{Collection} {
post {
result.getIDs().length == 0: "The created collection must be empty!"
result.getLength() == 0: "The created collection must be empty!"
}
}

Expand All @@ -260,24 +260,23 @@ access(all) contract NonFungibleToken {

/// deposit takes a NFT and adds it to the collections dictionary
/// and adds the ID to the id array
access(all) fun deposit(token: @{NonFungibleToken.NFT})
// {
// pre {
// // We emit the deposit event in the `Collection` interface
// // because the `Collection` interface is almost always the final destination
// // of tokens and deposit emissions from custom receivers could be confusing
// // and hard to reconcile to event listeners
// //NonFungibleToken.emitNFTDeposit(id: token.getID(), uuid: token.uuid, to: self.owner?.address, type: token.getType().identifier)
// }
// }
access(all) fun deposit(token: @{NonFungibleToken.NFT}) {
pre {
// We emit the deposit event in the `Collection` interface
// because the `Collection` interface is almost always the final destination
// of tokens and deposit emissions from custom receivers could be confusing
// and hard to reconcile to event listeners
NonFungibleToken.emitNFTDeposit(id: token.getID(), uuid: token.uuid, to: self.owner?.address, type: token.getType().identifier)
}
}

/// Function for a direct transfer instead of having to do a deposit and withdrawal
/// This can and should return false if the transfer doesn't succeed and true if it does succeed
///
access(Withdrawable) fun transfer(id: UInt64, receiver: Capability<&{NonFungibleToken.Receiver}>): Bool {
pre {
receiver.check(): "Could not borrow a reference to the NFT receiver"
//NonFungibleToken.emitNFTTransfer(id: id, uuid: self.borrowNFTSafe(id: id)?.uuid, from: self.owner?.address, to: receiver.borrow()?.owner?.address, type: self.borrowNFT(id).getType().identifier)
self.getIDs().contains(id): "The collection does not contain the specified ID"
Copy link
Member

Choose a reason for hiding this comment

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

This could be a problem because some collections are so large that they can't return the whole list of IDs because it goes over the gas limit. might need to remove this. lets try to get some other opinions though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. FWIW I think it's best to remove it to keep the interface flexible, but worth bringing up in the open house this week

Copy link
Collaborator Author

@sisyphusSmiling sisyphusSmiling Nov 7, 2023

Choose a reason for hiding this comment

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

If we want to proceed here, the problem then is in the NonFungibleToken.emitNFTTransfer(..., type: self.borrowNFT(id).getType().identifier). We'll need to determine if we can emit String? otherwise the self.getIDs().contains(id) pre-condition remains necessary. FWIW I'll add the condition back in and leave it as a point of discussion

NonFungibleToken.emitNFTTransfer(id: id, uuid: self.borrowNFTSafe(id: id)?.uuid, from: self.owner?.address, to: receiver.borrow()?.owner?.address, type: self.borrowNFT(id).getType().identifier)
}
}

Expand Down
Loading
Loading