Add batch calls for faster deployment #62
Conversation
use new message for each call added reentrancy flag spelling fix
crates/rmrk/src/batch.rs
Outdated
| RmrkError::InputVectorTooBig | ||
| ); | ||
| for token_id in tokens { | ||
| _ = MultiAsset::add_asset_to_token(self, token_id.clone(), asset_id, None); |
There was a problem hiding this comment.
Should we handle errors instead of ignoring with an empty assignment?
crates/rmrk/src/batch.rs
Outdated
|
|
||
| pub const MAX_BATCH_TOKENS_PER_ASSET: usize = 50; | ||
| pub const MAX_BATCH_ADD_CHILDREN: usize = 50; | ||
| pub const MAX_BATCH_TOKEN_TRANSFERS: usize = 50; |
There was a problem hiding this comment.
I'm inclined to say we remove max_batch limits as they are arbitrary and there's no way for the user to change this if we assume a wrong number.
There's no real danger here of the transaction trapping if the user provides an vec that is too large, as we are not storing anything. IMO we should leave this to the user to manage with dry-runs/etc rather than imposing an arbitrary limit.
crates/minting/src/lib.rs
Outdated
| @@ -91,6 +94,13 @@ where | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /// Assign metadata to specified token. | |||
| default fn transfer_token(&mut self, to: AccountId, id: Id, data: Vec<u8>) -> Result<()> { | |||
There was a problem hiding this comment.
Is this required? The user can use PSP34::transfer directly
There was a problem hiding this comment.
this was not reachable from ref call. Now it is calling psp34::internal
crates/rmrk/src/batch.rs
Outdated
| RmrkError::InputVectorTooBig | ||
| ); | ||
| for (token_id, destination) in token_to_destination { | ||
| _ = Minting::transfer_token(self, destination, token_id, Vec::new()); |
There was a problem hiding this comment.
Can we use PSP34::transfer?
crates/minting/src/lib.rs
Outdated
| /// Assign metadata to specified token. | ||
| default fn transfer_token(&mut self, to: AccountId, id: Id, data: Vec<u8>) -> Result<()> { |
There was a problem hiding this comment.
If this function is required please update the docs
crates/rmrk/src/batch.rs
Outdated
| RmrkError::InputVectorTooBig | ||
| ); | ||
| for (parent_id, child_id) in parent_child_pair { | ||
| _ = Nesting::add_child(self, parent_id, (child_contract, child_id)); |
There was a problem hiding this comment.
| _ = Nesting::add_child(self, parent_id, (child_contract, child_id)); | |
| Nesting::add_child(self, parent_id, (child_contract, child_id))?; |
|
|
||
| for (let i = 0; i < sets; i++) { | ||
| var tokenList = new Array(); | ||
| // console.log("addAssetToManyTokens, set=", i) |
There was a problem hiding this comment.
These commented-out logs should probably be removed.
For some of the calls the block POV size limit is easily reached. Implement batch calls for all functions which need the bulk calls for configuration and deployment.
It should not be mandatory to include this trait.
The example use of this trait is in the
example/equippable.Integration test is in
tests/batch.spec.ts.Acceptance criteria:
add_asset_to_many_tokens- with integration testadd_many_childrenwith integration testtransfer_manywith integration test