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

Implements BulkWriter.transferAndEquip. #269

Merged
merged 4 commits into from
Jul 5, 2024
Merged

Conversation

steven2308
Copy link
Member

Description

The NFT to transfer can be equipped, nested or free. The destination must be an NFT.
The NFT can be equipped into the destination.
If destination slot is occupied, it will be unequipped.
All these operations happen under a single transactions, given the right approvals.

Checklist

  • Verified code additions
  • Updated NatSpec comments (if applicable)
  • Regenerated docs
  • Ran prettier
  • Added tests to fully cover the changes

The NFT to transfer can be equipped, nested or free.
The destination must be an NFT.
The NFT can be equipped into the destination.
If destination slot is occupied, it will be unequipped.
All these operations happen under a single transactions, given the right approvals.
Copy link

openzeppelin-code bot commented Apr 19, 2024

Implements BulkWriter.transferAndEquip.

Generated at commit: 623fcc8d2abf01510ec190e4b6f22908ea3953db

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
0
0
5
27
34
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector


if (equipData.assetId == 0) return;

_unequipSlotIfNecessary(
Copy link
Member

Choose a reason for hiding this comment

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

wondering if there's any benefit in re-using replaceEquipement function from this contract here instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sound great, will give it a try.

This saves about 40% of gas since accept operation is not needed,
and pending children array is not used.

It also saves 4.5% of contract size.
@steven2308 steven2308 merged commit 789ed4b into dev Jul 5, 2024
4 checks passed
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.

2 participants