Skip to content

Conversation

@stefan-mysten
Copy link
Collaborator

This PR updates the codebase to use the new v2 faucet APIs.

@stefan-mysten stefan-mysten requested a review from bmwill as a code owner May 5, 2025 23:45
Comment on lines 572 to 573
client: &'a Client,
) -> (Address, Ed25519PrivateKey, Vec<Coin<'a>>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this lifetime is correct as value borrowed wouldn't live long enough. probably more correct to make this 'static

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clippy complains when making this static, here (and in all subsequent helper_setup calls)

let (address, pk, coins) = helper_setup(&mut tx, &client).await;

Copy link
Collaborator

Choose a reason for hiding this comment

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

whats the error you're seeing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I make Vec<Coin<'static>>, then we get this -- note that compiling is fine, but clippy is unhappy

error: lifetime may not live long enough
   --> crates/sui-transaction-builder/src/lib.rs:602:9
    |
570 |     async fn helper_setup<'a>(
    |                           -- lifetime `'a` defined here
...
602 |         (address, pk, coins.to_vec())
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'a` must outlive `'static`

Copy link
Collaborator Author

@stefan-mysten stefan-mysten May 16, 2025

Choose a reason for hiding this comment

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

Note, that I am still pretty unfamiliar with lifetime annotations and don't really understand what's going on in this case.

If I remove the <'a> lifetime annotation from the function and replace it like this:

    async fn helper_setup(
        tx: &mut TransactionBuilder,
        client: &'static Client,
    ) -> (Address, Ed25519PrivateKey, Vec<Coin<'static>>) {

then building fails, which is fair because all those tests create a local client object.

672 |         let client = Client::new_localhost();
   |             ------ binding `client` declared here
673 |         let (_, pk, coins) = helper_setup(&mut tx, &client).await;
   |                              ----------------------^^^^^^^-
   |                              |                     |
   |                              |                     borrowed value does not live long enough
   |                              argument requires that `client` is borrowed for `'static`
   ```

@bmwill bmwill merged commit 282157d into master May 16, 2025
6 checks passed
@bmwill bmwill deleted the faucet_v2 branch May 16, 2025 16:09
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.

3 participants