-
Notifications
You must be signed in to change notification settings - Fork 14
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
singular to batched state apis #133
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexytsu
force-pushed
the
nft/wip-3-batched
branch
from
October 11, 2022 04:25
12dc889
to
1f0dd0d
Compare
ZenGround0
approved these changes
Oct 14, 2022
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.
LGTM
/// Makes a transfer of a token from one address to another. The caller must verify that such a transfer is allowed. | ||
/// Makes a transfer of a token from one address to another. The caller must verify that such a | ||
/// transfer is allowed. This function does not flush the token AMT or the owner HAMT, it is the | ||
/// caller's responsibility to do so at the end of the batch. |
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.
👍
alexytsu
added a commit
that referenced
this pull request
Oct 19, 2022
* basic transfer logic * wip * allow account level delegation * token-level approvals * singular to batched state apis (#133) * rename account-level authorization methods * use vector of ActorIDs to record operators
alexytsu
added a commit
that referenced
this pull request
Nov 8, 2022
* basic transfer logic * wip * allow account level delegation * token-level approvals * singular to batched state apis (#133) * rename account-level authorization methods * use vector of ActorIDs to record operators
alexytsu
added a commit
that referenced
this pull request
Nov 17, 2022
* NFT Transfers (#130) * basic transfer logic * wip * allow account level delegation * token-level approvals * singular to batched state apis (#133) * rename account-level authorization methods * use vector of ActorIDs to record operators * NFT Receiver Hooks (#135) * add receiver hook support to state level * batch for minting * structure tests with logical blocks * generate return value from current state (#138) * State Invariant Checker for NFTs (#140) * added a state invariant checker for NFTs * delete map entries where possible during burn operations * update toolchain during build jobs * cargo clippy --fix * Multi actor integration tests (#141) * added a state invariant checker for NFTs * delete map entries where possible during burn operations * update toolchain during build jobs * cargo clippy --fix * metadataid to metadata * separate traits for testing frc46 and nft testing fixtures * rename test_actor to frc46_test_actor * test nft actor * actor to actor transfer tests * fix bug in operator burn * NFT Integration Tests (#143) * separate testing modules into separate files * mint to alice - alice rejects * mint to token contract itself * alice mints to herself - burns * mint to alice, hook transfers to bob * alice mints to bob who rejects * token contract transfer tests * two actor tests completed * fix dependencies * don't link nft_actor into integration tests * NFT improvements (#145) * add OwnerOf * annotate tests with post-run state * make metadata a string instead of a cid * use bitfield to store operators * temporary fix for yanked funty1.2.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes changes to have the APIs be batched
There's probably some more incremental improvements so that multiple state functions can be batched together without flushing the AMTs/HAMTs multiple times.
e.g. in the use case where multiple tokens are minted then approved for an operator at once, it would be nice for the
Mint
to simply take in &mut to the HAMTs and AMTs and have the caller flush at the end of the entire transaction.