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

Support retrieve last received object in Move tests #442

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Feb 11, 2022

Add a new test-only Move library function that allows us to retrieve the most recent object received through transfer by the current signer. This will allow us to inspect the received object without exiting Move, and unblock many ways to test move code.
Added a test.

Base automatically changed from update-move to main February 11, 2022 23:05
Copy link
Collaborator

@sblackshear sblackshear left a comment

Choose a reason for hiding this comment

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

So excited for the devx improvement this will bring!

Have some ideas about setting up the API's to allow testing more scenarios, but love the direction.


let signer = pop_arg!(args, Vec<u8>);

// TODO: what should the cost of this be?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think test-only functions should have 0 gas cost to perturb the gas results as little as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the TODO. The gas amount doesn't really matter here.

/// It can be from either a normal transfer or freeze_after_transfer.
/// This function can only be used in testing because any change to the object
/// could result in inconsistency with Sui storage in a prod system.
public fun get_last_received_object<T: key>(ctx: &TxContext): T {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • I think this would be more general if it took an address instead of TxContext. If we are trying to test a function that transfers to someone other than the tx sender, I believe we will need this.
  • Similarly, I think we will need to test functions/tx workflows that transfer >1 object. For this, perhaps we should also expose something like get_transfer<T: key>(n: u64): T that gives you the nth object transferred by this transaction.

To suggest one concrete combination of API's

#[test_only]
module FastX::TestTransfer {
/// Return the last object transferred by the current transaction
/// Aborts if the transaction has not transferred any objects or the last transfer does not have type `T`
public fun get_last_transfer<T: key>(): T

/// Return the last object transferred by the current transaction
/// Aborts if index is out of bounds or the `index`th transfer does not have type T
public fun get_transfer<T: key>(index: u64): T

/// Get the last object of type `T` transferred to address `a`. 
/// Aborts if there have been no transfers to `a`, or if the last transfer to `a` did not have type `T`
public fun get_last_transfer_to<T: key>(a: Address): T

/// Return the number of transfers performed by the current transaction
public fun num_transfers(): u64
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah makes sense. I will add more APIs in a separate PR.
(this PR is to support retrieve last received object)

Copy link
Collaborator

Choose a reason for hiding this comment

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

On board re adding the others later. But I think generalizing to

public fun get_last_transfer_to<T: key>(a: Address): T

(or similar) here probably makes sense? get_last_received_object<T: key>(ctx: &TxContext) seems a bit too narrow--I'm guessing most transfers will be to addresses other than the tx sender.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm not sure. My assumption for how people might write tests using this is that they want to consume / use the objects received in some way, and to do that they often need the TxContext. So a typical test will always start by creating a few TxContext, each representing an account, and from there the TxContext represents the address. Basically even though we don't have authentication in tests, we could still try to mimic it as much as possible through TxContext.

@lxfind lxfind force-pushed the support-retrieve-transferred-object branch 2 times, most recently from 7dd8a25 to 88a5b78 Compare February 12, 2022 07:08
@lxfind lxfind force-pushed the support-retrieve-transferred-object branch from 88a5b78 to 67a703c Compare February 12, 2022 22:25
@lxfind lxfind requested a review from sblackshear February 12, 2022 22:26
/// It can be from either a normal transfer or freeze_after_transfer.
/// This function can only be used in testing because any change to the object
/// could result in inconsistency with Sui storage in a prod system.
public fun get_last_received_object<T: key>(ctx: &TxContext): T {
Copy link
Collaborator

Choose a reason for hiding this comment

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

On board re adding the others later. But I think generalizing to

public fun get_last_transfer_to<T: key>(a: Address): T

(or similar) here probably makes sense? get_last_received_object<T: key>(ctx: &TxContext) seems a bit too narrow--I'm guessing most transfers will be to addresses other than the tx sender.

Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

🚀

@lxfind lxfind merged commit e4196d6 into main Feb 14, 2022
@lxfind lxfind deleted the support-retrieve-transferred-object branch February 14, 2022 16:25
@lxfind lxfind linked an issue Feb 15, 2022 that may be closed by this pull request
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.

Design a testing framework for Move code
3 participants