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

Use TransactionFactoryParameters in tests #11987

Conversation

kiminuo
Copy link
Collaborator

@kiminuo kiminuo commented Nov 23, 2023

Follow-up to #11738

Easier to review commit by commit.

Comment on lines -37 to -52
/// <exception cref="ArgumentException"/>
/// <exception cref="ArgumentNullException"/>
/// <exception cref="ArgumentOutOfRangeException"/>
public BuildTransactionResult BuildTransaction(
PaymentIntent payments,
FeeRate feeRate,
IEnumerable<OutPoint>? allowedInputs = null,
IPayjoinClient? payjoinClient = null,
Func<LockTime>? lockTimeSelector = null,
bool allowUnconfirmed = true,
bool allowDoubleSpend = false,
bool tryToSign = true)
{
TransactionParameters parameters = new(payments, feeRate, allowUnconfirmed, allowDoubleSpend, allowedInputs, tryToSign);
return BuildTransaction(parameters, lockTimeSelector, payjoinClient);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is sort of the main change of this PR.


namespace WalletWasabi.Tests.Helpers;

public class TransactionParametersBuilder
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This might get promoted from the test project to a different project if there is a for it need. I sort of thing there will be, but not just yet.

@kiminuo kiminuo marked this pull request as ready for review November 23, 2023 21:37
Comment on lines -21 to -32
public static PSBT GenerateRandomTransaction()
{
var key = new Key();
var tx =
Network.Main.CreateTransactionBuilder()
.AddCoins(Coin(0.5m, key.PubKey.GetScriptPubKey(ScriptPubKeyType.Segwit)))
.AddKeys(key)
.Send(BitcoinFactory.CreateScript(), Money.Coins(0.5m))
.BuildPSBT(true);
tx.Finalize();
return tx;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused method.

@@ -94,7 +82,11 @@ public void LazyPayjoinServerTest()
using Key key = new();
PaymentIntent payment = new(key.PubKey, amount);

var tx = transactionFactory.BuildTransaction(payment, new FeeRate(2m), allowedCoins.Select(x => x.Outpoint), payjoinClient);
Copy link
Collaborator Author

@kiminuo kiminuo Nov 24, 2023

Choose a reason for hiding this comment

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

Fee rate is very often 2, so that was put in the CreateBuilder() method.

@kiminuo kiminuo force-pushed the feature/2023-11-23-TransactionParameterBuilder branch from f3d8afb to 337cdb8 Compare November 24, 2023 08:37
@kiminuo kiminuo force-pushed the feature/2023-11-23-TransactionParameterBuilder branch from 337cdb8 to e023942 Compare November 24, 2023 08:37

public static SmartCoin[] CreateCoins(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Extracted from the CreateTransactionFactory method. I believe it will be useful down the line.

Copy link
Collaborator

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

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

Nice! Code LGTM.

Copy link
Collaborator

@Szpoti Szpoti left a comment

Choose a reason for hiding this comment

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

Code LGTM, tests are passing.

@adamPetho
Copy link
Collaborator

I think we can merge this.

@adamPetho adamPetho merged commit 1381815 into WalletWasabi:master Nov 24, 2023
6 of 7 checks passed
@kiminuo kiminuo deleted the feature/2023-11-23-TransactionParameterBuilder branch November 24, 2023 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants