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

Send transactions and transfer funds between wallets #110

Merged
merged 10 commits into from
Feb 23, 2022
Merged

Conversation

digorithm
Copy link
Member

@digorithm digorithm commented Feb 23, 2022

Closes #68.
Closes #31.

Initially, this was targeting only transaction sending, but the use case I was solving for was... sending coins, so one thing led to another, and I needed to have a proper coin selecting API in place first. So I bundled it all together in this PR.

This enables not only sending signed transactions but also adds an API in the wallet/signer for sending coins between wallets:

// Setup two sets of coins, one for each wallet, each containing 1 coin with 1 amount.
let (pk_1, mut coins_1) = setup_address_and_coins(1, 1);
let (pk_2, coins_2) = setup_address_and_coins(1, 1);

coins_1.extend(coins_2);

// Setup a provider and node with both set of coins
let provider = setup_test_provider(coins_1).await;

let wallet_1 = LocalWallet::new_from_private_key(pk_1, provider.clone()).unwrap();
let wallet_2 = LocalWallet::new_from_private_key(pk_2, provider).unwrap();

let wallet_1_initial_coins = wallet_1.get_coins().await.unwrap();
let wallet_2_initial_coins = wallet_2.get_coins().await.unwrap();

// Check initial wallet state
assert_eq!(wallet_1_initial_coins.len(), 1);
assert_eq!(wallet_2_initial_coins.len(), 1);

// Transfer 1 from wallet 1 to wallet 2
let _receipts = wallet_1
    .transfer(&wallet_2.address(), 1, Default::default())
    .await
    .unwrap();

// Currently ignoring the effect on wallet 1, as coins aren't being marked as spent for now
let _wallet_1_final_coins = wallet_1.get_coins().await.unwrap();
let wallet_2_final_coins = wallet_2.get_coins().await.unwrap();

// Check that wallet two now has two coins
assert_eq!(wallet_2_final_coins.len(), 2);

// Transferring more than balance should fail
let result = wallet_1
    .transfer(&wallet_2.address(), 2, Default::default())
    .await;

assert!(result.is_err());
let wallet_2_coins = wallet_2.get_coins().await.unwrap();
assert_eq!(wallet_2_coins.len(), 2); // Not changed

It also handles Output::Change. Note that the sent UTXOs aren't marked as spent because that depends on a work in progress on fuel-core.

Note for when this is merged: add examples in the Sway book showing this functionality.

@digorithm
Copy link
Member Author

@Voxelot adding you as a reviewer for this one because it uses some new stuff introduced in fuel-core and it implements some stuff we discussed recently (UTXOs, etc). Would appreciate your review of it.

@digorithm digorithm self-assigned this Feb 23, 2022
@digorithm digorithm added the enhancement New feature or request label Feb 23, 2022
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

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

I feel like we could use some more unit tests. Otherwise LGTM!

fuels-signers/src/provider.rs Outdated Show resolved Hide resolved
fuels-signers/src/provider.rs Show resolved Hide resolved
fuels-signers/src/wallet/mod.rs Outdated Show resolved Hide resolved
iqdecay
iqdecay previously approved these changes Feb 23, 2022
fuels-signers/Cargo.toml Outdated Show resolved Hide resolved
fuels-signers/Cargo.toml Outdated Show resolved Hide resolved
Voxelot
Voxelot previously approved these changes Feb 23, 2022
iqdecay
iqdecay previously approved these changes Feb 23, 2022
@digorithm digorithm dismissed stale reviews from iqdecay and Voxelot via c79f0f1 February 23, 2022 20:41
@Voxelot
Copy link
Member

Voxelot commented Feb 23, 2022

Eventually, we'll need a way to track which coins are on a pending tx and exclude them from the coin selection API.. This is likely a concern we can handle entirely on the fuel-core side though.

@digorithm
Copy link
Member Author

Eventually, we'll need a way to track which coins are on a pending tx and exclude them from the coin selection API..

Would that be happening in the client itself? i.e. when executing a method from the coin selection API it would check for the pending TX and not include that coin when returning the result to the caller?

@Voxelot
Copy link
Member

Voxelot commented Feb 23, 2022

Yeah I'm assuming we could query the txpool from the graphql API handler and filter any coins that are being tracked in that dependency graph.

@digorithm
Copy link
Member Author

Yeah, sounds like a good plan. Do we have an issue for that already?

@Voxelot
Copy link
Member

Voxelot commented Feb 23, 2022

Just added: FuelLabs/fuel-core#185

@Voxelot
Copy link
Member

Voxelot commented Feb 23, 2022

The sdk might also want to track recently used utxos, just in case a series of requests to coins_to_spend gets round-robined between multiple nodes before their txpools sync up. In which case, maybe the coins_to_spend could take a list of utxos to exclude from the search provided by the sdk.

@digorithm
Copy link
Member Author

The sdk might also want to track recently used utxos, just in case a series of requests to coins_to_spend gets round-robined between multiple nodes before their txpools sync up. In which case, maybe the coins_to_spend could take a list of utxos to exclude from the search provided by the sdk.

That makes sense, let's track that on a separate issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Create an interface in Wallet to send transactions Coin selection API
3 participants