Comments (7)
@giansalex thanks for the detailed write up of this. i appreciate the simple example, and link to the code. i'll get to this early next week likely.
from gon-evidence.
https://github.com/cosmos/ibc/tree/main/spec/app/ics-721-nft-transfer#packet-relay
- When a packet times out, tokens represented in the packet are either unescrowed or minted back to the sender appropriately -- depending on whether the tokens are being moved away from or back toward their source.
According to this, it is understood that the token must have been burned when it was sent.
from gon-evidence.
reposting my thoughts on this from the Discord:
I feel that the CW implementation is behaving correctly and in line with the spec here. the state machine is:
- send,
- on error, return
- on success, burn
the SDK module approach is:
- send & burn
- on error, re-mint
- on success, do nothing
the trouble with this is that the NFT is destroyed before it is confirmed to have been transferred! this feels like an invariant violation to me. imagine if other code performed an action when the NFT is destroyed - the author would probably be surprised to learn that the NFT can be re-minted. we are building a bridge, so in my view we should prioritise correctness over all else.
additionally, NFTs are not locked by the CW approach, you just need to self-relay your ACK, in addition to the packet. If you do that, it will happily burn the NFT on completion.
from gon-evidence.
@giansalex, what's your opinion on how the spec should work? it feels strange to me to allow transferring a NFT that has not been ACK'd as being received yet.
from gon-evidence.
https://github.com/cosmos/ibc/tree/main/spec/app/ics-721-nft-transfer#packet-relay
- When a packet times out, tokens represented in the packet are either unescrowed or minted back to the sender appropriately -- depending on whether the tokens are being moved away from or back toward their source.
According to this, it is understood that the token must have been burned when it was sent.
This is a heavy bug and exploit. Check my explanation here: #705 (comment)
The issue you are pointing out is due to congestions by IBC messages relaying issues, where ACK success and fail messages are delivered very late due to mass amount of packets (e.g. by vector attacks). IBC though guarantees sending ACKs - no matter how long it takes. Only solution here is to self-relay or wait :). But for ICS721 transfer it is good that NFT is locked until then.
from gon-evidence.
This is a heavy bug and exploit. Check my explanation here: #705 (comment)
can you demonstrate this exploit on the chain?
My comment: #705 (comment)
from gon-evidence.
@giansalex, what's your opinion on how the spec should work? it feels strange to me to allow transferring a NFT that has not been ACK'd as being received yet.
sorry, I had not seen this.
I agree with the current implementation, generally nft-transfers will be successful, ACK+error and Timeout events occur rarely or exceptionally (in testnet occurred due to a malicious contract attack).
it is also more optimal for the relayers, since the gas consumed in the NFT burning will be charged to the user (nft-transfer Tx)
from gon-evidence.
Related Issues (20)
- AWARD: Disperze Claim 3
- AWARD: Disperze Claim 5
- CLOSED
- AWARD: keyllli223 Claim 2
- AWARD: DAICers Claim 6
- AWARD: Xavier Team Claim 4 HOT 1
- AWARD: Ankaria Claim "6 - Best NFT Memes"
- AWARD: silent Claim 6
- AWARD: Stakeflow Claim 2 HOT 2
- AWARD: NSeven Projects Claim 4
- AWARD: LIVE.Node Claim 5
- AWARD: LIVE.Node Claim 6
- closed
- AWARD: Power//Team Claim 6 - MEME
- AWARD:xbdyhh Claim 6 - MEME
- AWARD:DreamGallery Claim 6 - MEME
- AWARD: Ping.pub Claim "2 - Public Good Awards"
- AWARD: NSeven Projects Claim 6
- AWARD: Disperze Claim 5
- AWARD: Disperze Claim 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gon-evidence.