feat: ERC1155 #275
feat: ERC1155 #275
Conversation
✅ Deploy Preview for contracts-stylus canceled.
|
There was a problem hiding this comment.
You are doing great job @programskillforverification! 🚀
The structure and design look good.
Remember to add antora docs as here - https://github.com/OpenZeppelin/rust-contracts-stylus/tree/v0.1.0/docs/modules/ROOT/pages
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
Thanks for your review, I will fix all asap |
programskillforverification
force-pushed
the
erc1155
branch
from
534a917
to
b136375
Compare
|
A developer is asking for an ERC1155 Stylus Rust implementation in @arbitrum_stylus TG chat. |
Would love to hear some expected timelines but I could still use this as inspiration for my own implementation. |
|
@bidzyyys is ERC1155 Stylus Rust implementation urgent? If so, I would to try the best finish all but antora docs by this week. |
There was a problem hiding this comment.
Great job @bidzyyys!
Thank you for pulling it up.
I think we are good to merge it.
Few minor nits I have:
contracts/src/token/erc1155/mod.rs
Outdated
| /// # Arguments | ||
| /// | ||
| /// * `&self` - Read access to the contract's state. | ||
| /// * `owner` - Account of the token's owner. |
There was a problem hiding this comment.
| /// * `owner` - Account of the token's owner. | |
| /// * `account` - Account of the token's owner. |
I may be out of the loop and you agreed this nit shouldn't hold the PR from being merged, but I have to do my due diligence - the wrong argument is still being described.
| values, | ||
| data, | ||
| )?; | ||
| Ok(()) |
There was a problem hiding this comment.
nit: ? and Ok(()) can be omitted, since _update_with_acceptance_check's result (also Ok(())) can be propagated; same goes for a couple of other places
Don't have to change this, just noting
| error ERC1155InvalidOperator(address operator); | ||
|
|
||
| /// Indicates an array length mismatch between token ids and values in a | ||
| /// [`IErc1155::safe_batch_transfer_from`] operation. |
There was a problem hiding this comment.
Technically speaking, safe_batch_transfer_from is not the only operation that can return this error; it is returned in all operations that utilize fn require_equal_arrays_length, which are:
balance_of_batch_mint_batch_mintsafe_transfer_fromsafe_batch_transfer_from_burn_burn_batch
The original comment from the Solidity version is therefore technically incorrect as well.
| contract | ||
| .set_approval_for_all(BOB, true) | ||
| .expect("should approve Bob for operations on all Alice's tokens"); | ||
| assert_eq!(contract.is_approved_for_all(alice, BOB), true); |
There was a problem hiding this comment.
This isn't a big deal, but all equality assertions between two boolean values could be shortened if just the regular assert!(...) is used.
assert!(contract.is_approved_for_all(alice, BOB)); // assert true
assert!(!contract.is_approved_for_all(alice, BOB)); // assert false| #[motsu::test] | ||
| fn set_approval_for_all(contract: Erc1155) { | ||
| let alice = msg::sender(); | ||
| contract._operator_approvals.setter(alice).setter(BOB).set(false); |
There was a problem hiding this comment.
This should already be false by default
Resolves #261
PR Checklist