-
Notifications
You must be signed in to change notification settings - Fork 25
Simplify the way that signatures are collected #283
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
Open
quantumagi
wants to merge
14
commits into
stratisproject:master
Choose a base branch
from
quantumagi:simplify
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
f58eb26
Add SignTransaction helper method to FederationWalletManager
quantumagi 489f66d
Add SignatureProvider to be used in API call
quantumagi f4e6c4d
Add AuthorizeWithdrawals API to gateway controller
quantumagi df2b548
Update SignTransaction method
quantumagi 4d2d0b4
Add unit test
quantumagi 3efdfca
Refactor to reduce overhead of key decryption
quantumagi 2756fb2
Improve logging
quantumagi 620635f
Update test
quantumagi 0432681
Revert to one transaction per call
quantumagi ea053f2
Add TODO
quantumagi e1dbf16
Fix test
quantumagi e2122b6
Small typo
quantumagi dfe90eb
Add comments
quantumagi 19bc7b0
Revisit class comment
quantumagi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
7 changes: 7 additions & 0 deletions
7
src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/IAuthorizeWithdrawalsModel.cs
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| namespace Stratis.FederatedPeg.Features.FederationGateway.Interfaces | ||
| { | ||
| public interface IAuthorizeWithdrawalsModel | ||
| { | ||
| string TransactionHex { get; } | ||
| } | ||
| } |
This file contains hidden or 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
19 changes: 19 additions & 0 deletions
19
src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ISignatureProvider.cs
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| namespace Stratis.FederatedPeg.Features.FederationGateway.Interfaces | ||
| { | ||
| public interface ISignatureProvider | ||
| { | ||
| /// <summary> | ||
| /// Signs an externally provided withdrawal transaction if it is deemed valid. This method | ||
| /// is used to sign a transaction in response to signature requests from the federation | ||
| /// leader. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This method requires federation to be active as the wallet password is supplied during | ||
| /// activation. Transaction's are validated to ensure that they are expected as per | ||
| /// the deposits received on the source chain. | ||
| /// </remarks> | ||
| /// <param name="transactionHex">The hexadecimal representations of transactions to sign.</param> | ||
| /// <returns>An array of signed transactions (in hex) or <c>null</c> for transactions that can't be signed.</returns> | ||
| string SignTransaction(string transactionHex); | ||
| } | ||
| } | ||
16 changes: 16 additions & 0 deletions
16
src/Stratis.FederatedPeg.Features.FederationGateway/Models/AuthorizeWithdrawalsModel.cs
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
| using Newtonsoft.Json; | ||
| using Stratis.Bitcoin.Features.Wallet.Models; | ||
| using Stratis.FederatedPeg.Features.FederationGateway.Interfaces; | ||
|
|
||
| namespace Stratis.FederatedPeg.Features.FederationGateway.Models | ||
| { | ||
| /// <summary> | ||
| /// An instance of this class represents a particular block hash and associated height on the source chain. | ||
| /// </summary> | ||
| public class AuthorizeWithdrawalsModel : RequestModel, IAuthorizeWithdrawalsModel | ||
| { | ||
| [Required(ErrorMessage = "The transaction to authorize")] | ||
| public string TransactionHex { get; set; } | ||
| } | ||
| } |
105 changes: 105 additions & 0 deletions
105
src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/SignatureProvider.cs
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| using System.Linq; | ||
| using Microsoft.Extensions.Logging; | ||
| using NBitcoin; | ||
| using Stratis.Bitcoin.Utilities; | ||
| using Stratis.FederatedPeg.Features.FederationGateway.Interfaces; | ||
| using Stratis.FederatedPeg.Features.FederationGateway.Wallet; | ||
|
|
||
| namespace Stratis.FederatedPeg.Features.FederationGateway.TargetChain | ||
| { | ||
| /// <summary> | ||
| /// The purpose of this class is to sign externally provided withdrawal transactions if they are | ||
| /// deemed valid. Transactions would typically be signed in response to signature requests from | ||
| /// the federation leader. The federation is required to be active as the wallet password is | ||
| /// supplied during activation. Transaction's are validated to ensure that they are expected as | ||
| /// per the deposits received on the source chain. Out-of-sequence transactions or transactions | ||
| /// that are not utilising the expected UTXOs will not be signed. | ||
| /// </summary> | ||
| public class SignatureProvider: ISignatureProvider | ||
| { | ||
| private readonly IFederationWalletManager federationWalletManager; | ||
| private readonly ICrossChainTransferStore crossChainTransferStore; | ||
| private readonly IFederationGatewaySettings federationGatewaySettings; | ||
| private readonly Network network; | ||
| private readonly ILogger logger; | ||
|
|
||
| public SignatureProvider( | ||
| IFederationWalletManager federationWalletManager, | ||
| ICrossChainTransferStore crossChainTransferStore, | ||
| IFederationGatewaySettings federationGatewaySettings, | ||
| Network network, | ||
| ILoggerFactory loggerFactory) | ||
| { | ||
| Guard.NotNull(federationWalletManager, nameof(federationWalletManager)); | ||
| Guard.NotNull(crossChainTransferStore, nameof(crossChainTransferStore)); | ||
| Guard.NotNull(federationGatewaySettings, nameof(federationGatewaySettings)); | ||
| Guard.NotNull(network, nameof(network)); | ||
| Guard.NotNull(loggerFactory, nameof(loggerFactory)); | ||
|
|
||
| this.logger = loggerFactory.CreateLogger(this.GetType().FullName); | ||
| this.federationWalletManager = federationWalletManager; | ||
| this.federationGatewaySettings = federationGatewaySettings; | ||
| this.crossChainTransferStore = crossChainTransferStore; | ||
| this.network = network; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines if a withdrawal transaction can be authorized. | ||
| /// </summary> | ||
| /// <param name="transaction">The transaction to authorize.</param> | ||
| /// <param name="withdrawal">The withdrawal transaction already extracted from the transaction.</param> | ||
| /// <returns><c>True</c> if the withdrawal is valid and <c>false</c> otherwise.</returns> | ||
| private bool IsAuthorized(Transaction transaction, IWithdrawal withdrawal) | ||
| { | ||
| // It must be a transfer that we know about. | ||
| ICrossChainTransfer crossChainTransfer = this.crossChainTransferStore.GetAsync(new[] { withdrawal.DepositId }).GetAwaiter().GetResult().FirstOrDefault(); | ||
| if (crossChainTransfer == null) | ||
| return false; | ||
|
|
||
| // If its already been seen in a block then we probably should not authorize it. | ||
| if (crossChainTransfer.Status == CrossChainTransferStatus.SeenInBlock) | ||
| return false; | ||
|
|
||
| // The templates must match what we expect to see. | ||
| if (!CrossChainTransfer.TemplatesMatch(crossChainTransfer.PartialTransaction, transaction)) | ||
| return false; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private Transaction SignTransaction(Transaction transaction, Key key) | ||
| { | ||
| return this.federationWalletManager.SignTransaction(transaction, IsAuthorized, key); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public string SignTransaction(string transactionHex) | ||
| { | ||
| Guard.NotNull(transactionHex, nameof(transactionHex)); | ||
|
|
||
| this.logger.LogTrace("():{0}", transactionHex); | ||
|
|
||
| FederationWallet wallet = this.federationWalletManager.GetWallet(); | ||
| if (wallet == null || this.federationWalletManager.Secret == null) | ||
| { | ||
| this.logger.LogTrace("(-)[FEDERATION_INACTIVE]"); | ||
| return null; | ||
| } | ||
|
|
||
| Key key = wallet.MultiSigAddress.GetPrivateKey(wallet.EncryptedSeed, this.federationWalletManager.Secret.WalletPassword, this.network); | ||
| if (key.PubKey.ToHex() != this.federationGatewaySettings.PublicKey) | ||
| { | ||
| this.logger.LogTrace("(-)[FEDERATION_KEY_INVALID]"); | ||
| return null; | ||
| } | ||
|
|
||
| Transaction transaction = this.network.CreateTransaction(transactionHex); | ||
|
|
||
| transactionHex = SignTransaction(transaction, key)?.ToHex(this.network); | ||
|
|
||
| this.logger.LogTrace("(-):{0}", transactionHex); | ||
|
|
||
| return transactionHex; | ||
| } | ||
| } | ||
| } |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
Please describe the purpose and behaviour of this class in details, its clear it provides signatures but from who and how are the keys provided?
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.
The keys are provided by the API call that activates federation. The purpose of the class is in the associated issue. I will add the detail to the class.