-
-
Notifications
You must be signed in to change notification settings - Fork 364
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
feat: Reuse unused tokens for the next offer #11204
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's definitely a good idea to reuse the unused offer token. However, it need a better way to get the unused nft id.
Let me provide an example showing how it may cause chaotic situations.
- In the beginning you have an expired offer
[ { offerId: 1, nftSn: 1, status: EXPIRED }]
- Then you try to create a new offer and reuse the unused token
[ { offerId: 1, nftSn: 1, status: EXPIRED },
{ offerId: 2, nftSn: 1, status: ACTIVE }]
- If you continue to create a new offer
[ { offerId: 1, nftSn: 1, status: EXPIRED },
{ offerId: 2, nftSn: 1, status: ACTIVE },
{ offerId: 3, nftSn: 1, status: ACTIVE }]
In this step, there is already a problem occurred. You may create infinite offer with one nft.
- More chaos is coming If the offer 2 has been accepted.
[ { offerId: 1, nftSn: 1, status: EXPIRED },
{ offerId: 2, nftSn: 1, status: ACCEPT },
{ offerId: 3, nftSn: 1, status: ACTIVE }]
If you continue to creating a new offer, the current implement will still using nftSn: 1
.
However, the nftSn: 1
is not available anymore.
This step would cause problems again.
[ { offerId: 1, nftSn: 1, status: EXPIRED },
{ offerId: 2, nftSn: 1, status: ACCEPT },
{ offerId: 3, nftSn: 1, status: ACTIVE }
{ offerId: 4, nftSn: 1, status: ACTIVE }]
Let me know if this example is not clear or it's not possible to happen with the current implement.
the example you provided would defiantly be an issue if each new atomic swap mapped to an offer in our indexer, which it does not an offer's so when we create a new offer, with a reused token it will override the old offer code
so basically atm you can't have multiple swaps/offers with the same offered item this is either a bug or by design, cc @vikiival if it's the former I will make the required changes , otherwise the current implementation should pass the provided case. |
Yup, that's how it is implemented Swap is stored inside a map by key -
So yes each, one offered item per desired item |
PR Type
Context
let's save this cost to the user, by reusing unsued tokens from old offers
Screenshot 📸