PaimaStudios / paima-engine

Novel trustless web3/blockchain gaming engine.
MIT License
54 stars 20 forks source link

Add ERC721 transfer CDE #242

Open SebastienGllmt opened 1 year ago

SebastienGllmt commented 1 year ago

Right now we have an ERC721 CDE, but it only keeps track of new NFT mints

For transfers, we're instead relying on the generic CDE using something like this

abiPath: "./abis/ERC721.json"
eventSignature: "Transfer(address,address,uint256)"
scheduledPrefix: tradeTransfer

A good example of wanting to use this is the card game template we wanted to update the state machine when an NFT got burned (transferred to the 0 address). You can see documentation for that here

But it seems reasonable to want to have built-in CDE option for this (and maybe for other stds like ERC20, or even maybe a CDE for just regular ETH send instead of having to use a generic CDE over the GenericPayment contract)

I am a little concerned that we might might end up creating too many CDEs that are just shims over a Ethereum log event though.

Additionally, it's kind of strange that the erc721 CDE does keep track of transfers (and not just mints) internally to power utility functions like getNftOwner. I feels like the separation of concerns is not super clear (and this applies to a lesser extent with the ERC20 CDEs)

SebastienGllmt commented 3 months ago

To add to this, it would make sense to rename scheduledPrefix here to something like mintPrefix and then make it optional, just like we have burnScheduledPrefix optional