-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[Neo Core Exception] Create WalletException and use it to replace all exceptions in wallet #3434
Open
Jim8y
wants to merge
22
commits into
neo-project:master
Choose a base branch
from
Jim8y:standardize-exception
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 5 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
d12d5e6
standardize exception
Jim8y c4cfbc1
upate mac
Jim8y c5719a3
wrap methods in WalletException.
Jim8y 3285c9a
Merge branch 'master' into standardize-exception
shargon 58a3d23
Update tests/Neo.UnitTests/Wallets/UT_AssetDescriptor.cs
shargon f366801
One file per class
shargon bfd0219
check and fix all exception messages.
Jim8y bc1667c
Merge branch 'master' into standardize-exception
Jim8y 2dc9fb7
Merge branch 'master' into standardize-exception
Jim8y 25afa01
Merge branch 'master' into standardize-exception
cschuchardt88 3625988
Merge branch 'master' into standardize-exception
Jim8y 21c0d99
Update src/Neo/Wallets/Wallet.cs
Jim8y b43327b
fix
Jim8y 50c376a
Merge branch 'master' into standardize-exception
Jim8y 18b3ac9
Merge branch 'master' into standardize-exception
Jim8y 7bded56
Clean using
shargon bca2ff6
Merge branch 'master' into standardize-exception
Jim8y 84b4555
less try
Jim8y 472122c
do 30
Jim8y 449adc0
Merge branch 'standardize-exception' of github.com:Jim8y/neo into sta…
Jim8y 3baffac
Merge branch 'master' into standardize-exception
Jim8y 1ebd9f3
Merge branch 'master' into standardize-exception
Jim8y 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 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 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 |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
// Redistribution and use in source and binary forms with or without | ||
// modifications are permitted. | ||
|
||
using Akka.Routing; | ||
shargon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
using Neo.Cryptography; | ||
using Neo.IO; | ||
using Neo.Network.P2P; | ||
|
@@ -36,7 +37,14 @@ public static class Helper | |
/// <returns>The signature for the <see cref="IVerifiable"/>.</returns> | ||
public static byte[] Sign(this IVerifiable verifiable, KeyPair key, uint network) | ||
{ | ||
return Crypto.Sign(verifiable.GetSignData(network), key.PrivateKey); | ||
try | ||
{ | ||
return Crypto.Sign(verifiable.GetSignData(network), key.PrivateKey); | ||
} | ||
catch (Exception ex) | ||
{ | ||
throw WalletException.FromException(ex); | ||
} | ||
} | ||
|
||
/// <summary> | ||
|
@@ -61,17 +69,24 @@ public static string ToAddress(this UInt160 scriptHash, byte version) | |
/// <returns>The converted script hash.</returns> | ||
public static UInt160 ToScriptHash(this string address, byte version) | ||
{ | ||
byte[] data = address.Base58CheckDecode(); | ||
if (data.Length != 21) | ||
throw new FormatException(); | ||
if (data[0] != version) | ||
throw new FormatException(); | ||
return new UInt160(data.AsSpan(1)); | ||
try | ||
{ | ||
byte[] data = address.Base58CheckDecode(); | ||
if (data.Length != 21) | ||
throw new WalletException(WalletErrorType.FormatError, "Invalid address format: incorrect length"); | ||
if (data[0] != version) | ||
throw new WalletException(WalletErrorType.FormatError, "Invalid address version"); | ||
return new UInt160(data.AsSpan(1)); | ||
} | ||
catch (Exception e) when (e is not WalletException) | ||
{ | ||
throw new WalletException(WalletErrorType.FormatError, "Invalid address format"); | ||
} | ||
} | ||
|
||
internal static byte[] XOR(byte[] x, byte[] y) | ||
{ | ||
if (x.Length != y.Length) throw new ArgumentException(); | ||
if (x.Length != y.Length) throw new WalletException(WalletErrorType.InvalidOperation, "Arrays must have the same length"); | ||
byte[] r = new byte[x.Length]; | ||
for (int i = 0; i < r.Length; i++) | ||
r[i] = (byte)(x[i] ^ y[i]); | ||
|
@@ -90,78 +105,90 @@ internal static byte[] XOR(byte[] x, byte[] y) | |
/// <returns>The network fee of the transaction.</returns> | ||
public static long CalculateNetworkFee(this Transaction tx, DataCache snapshot, ProtocolSettings settings, Func<UInt160, byte[]> accountScript, long maxExecutionCost = ApplicationEngine.TestModeGas) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will make conflicts into my never ending pull request #3385 :'( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
{ | ||
UInt160[] hashes = tx.GetScriptHashesForVerifying(snapshot); | ||
|
||
// base size for transaction: includes const_header + signers + attributes + script + hashes | ||
int size = Transaction.HeaderSize + tx.Signers.GetVarSize() + tx.Attributes.GetVarSize() + tx.Script.GetVarSize() + IO.Helper.GetVarSize(hashes.Length), index = -1; | ||
uint exec_fee_factor = NativeContract.Policy.GetExecFeeFactor(snapshot); | ||
long networkFee = 0; | ||
foreach (UInt160 hash in hashes) | ||
try | ||
{ | ||
index++; | ||
byte[] witnessScript = accountScript(hash); | ||
byte[] invocationScript = null; | ||
UInt160[] hashes = tx.GetScriptHashesForVerifying(snapshot); | ||
|
||
if (tx.Witnesses != null && witnessScript is null) | ||
// base size for transaction: includes const_header + signers + attributes + script + hashes | ||
int size = Transaction.HeaderSize + tx.Signers.GetVarSize() + tx.Attributes.GetVarSize() + tx.Script.GetVarSize() + IO.Helper.GetVarSize(hashes.Length), index = -1; | ||
uint exec_fee_factor = NativeContract.Policy.GetExecFeeFactor(snapshot); | ||
long networkFee = 0; | ||
foreach (UInt160 hash in hashes) | ||
{ | ||
// Try to find the script in the witnesses | ||
Witness witness = tx.Witnesses[index]; | ||
witnessScript = witness?.VerificationScript.ToArray(); | ||
index++; | ||
byte[] witnessScript = accountScript(hash); | ||
byte[] invocationScript = null; | ||
|
||
if (witnessScript is null || witnessScript.Length == 0) | ||
if (tx.Witnesses != null && witnessScript is null) | ||
{ | ||
// Then it's a contract-based witness, so try to get the corresponding invocation script for it | ||
invocationScript = witness?.InvocationScript.ToArray(); | ||
} | ||
} | ||
// Try to find the script in the witnesses | ||
Witness witness = tx.Witnesses[index]; | ||
witnessScript = witness?.VerificationScript.ToArray(); | ||
|
||
if (witnessScript is null || witnessScript.Length == 0) | ||
{ | ||
var contract = NativeContract.ContractManagement.GetContract(snapshot, hash); | ||
if (contract is null) | ||
throw new ArgumentException($"The smart contract or address {hash} is not found"); | ||
var md = contract.Manifest.Abi.GetMethod(ContractBasicMethod.Verify, ContractBasicMethod.VerifyPCount); | ||
if (md is null) | ||
throw new ArgumentException($"The smart contract {contract.Hash} haven't got verify method"); | ||
if (md.ReturnType != ContractParameterType.Boolean) | ||
throw new ArgumentException("The verify method doesn't return boolean value."); | ||
if (md.Parameters.Length > 0 && invocationScript is null) | ||
throw new ArgumentException("The verify method requires parameters that need to be passed via the witness' invocation script."); | ||
if (witnessScript is null || witnessScript.Length == 0) | ||
{ | ||
// Then it's a contract-based witness, so try to get the corresponding invocation script for it | ||
invocationScript = witness?.InvocationScript.ToArray(); | ||
} | ||
} | ||
if (witnessScript is null || witnessScript.Length == 0) | ||
{ | ||
var contract = NativeContract.ContractManagement.GetContract(snapshot, hash); | ||
if (contract is null) | ||
throw new WalletException(WalletErrorType.ContractNotFound, $"The smart contract or address {hash} is not found"); | ||
var md = contract.Manifest.Abi.GetMethod(ContractBasicMethod.Verify, ContractBasicMethod.VerifyPCount); | ||
if (md is null) | ||
throw new WalletException(WalletErrorType.ContractError, $"The smart contract {contract.Hash} hasn't got a verify method"); | ||
if (md.ReturnType != ContractParameterType.Boolean) | ||
throw new WalletException(WalletErrorType.ContractError, "The verify method doesn't return boolean value"); | ||
if (md.Parameters.Length > 0 && invocationScript is null) | ||
throw new WalletException(WalletErrorType.ContractError, "The verify method requires parameters that need to be passed via the witness' invocation script"); | ||
|
||
// Empty verification and non-empty invocation scripts | ||
var invSize = invocationScript?.GetVarSize() ?? Array.Empty<byte>().GetVarSize(); | ||
size += Array.Empty<byte>().GetVarSize() + invSize; | ||
// Empty verification and non-empty invocation scripts | ||
var invSize = invocationScript?.GetVarSize() ?? Array.Empty<byte>().GetVarSize(); | ||
size += Array.Empty<byte>().GetVarSize() + invSize; | ||
|
||
// Check verify cost | ||
using ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, tx, snapshot.CloneCache(), settings: settings, gas: maxExecutionCost); | ||
engine.LoadContract(contract, md, CallFlags.ReadOnly); | ||
if (invocationScript != null) engine.LoadScript(invocationScript, configureState: p => p.CallFlags = CallFlags.None); | ||
if (engine.Execute() == VMState.FAULT) throw new ArgumentException($"Smart contract {contract.Hash} verification fault."); | ||
if (!engine.ResultStack.Pop().GetBoolean()) throw new ArgumentException($"Smart contract {contract.Hash} returns false."); | ||
// Check verify cost | ||
using ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, tx, snapshot.CloneCache(), settings: settings, gas: maxExecutionCost); | ||
engine.LoadContract(contract, md, CallFlags.ReadOnly); | ||
if (invocationScript != null) engine.LoadScript(invocationScript, configureState: p => p.CallFlags = CallFlags.None); | ||
if (engine.Execute() == VMState.FAULT) throw new WalletException(WalletErrorType.ExecutionFault, $"Smart contract {contract.Hash} verification fault"); | ||
if (!engine.ResultStack.Pop().GetBoolean()) throw new WalletException(WalletErrorType.VerificationFailed, $"Smart contract {contract.Hash} returns false"); | ||
|
||
maxExecutionCost -= engine.FeeConsumed; | ||
if (maxExecutionCost <= 0) throw new InvalidOperationException("Insufficient GAS."); | ||
networkFee += engine.FeeConsumed; | ||
} | ||
else if (IsSignatureContract(witnessScript)) | ||
{ | ||
size += 67 + witnessScript.GetVarSize(); | ||
networkFee += exec_fee_factor * SignatureContractCost(); | ||
maxExecutionCost -= engine.FeeConsumed; | ||
if (maxExecutionCost <= 0) throw new WalletException(WalletErrorType.InsufficientFunds, "Insufficient GAS"); | ||
networkFee += engine.FeeConsumed; | ||
} | ||
else if (IsSignatureContract(witnessScript)) | ||
{ | ||
size += 67 + witnessScript.GetVarSize(); | ||
networkFee += exec_fee_factor * SignatureContractCost(); | ||
} | ||
else if (IsMultiSigContract(witnessScript, out int m, out int n)) | ||
{ | ||
int size_inv = 66 * m; | ||
size += IO.Helper.GetVarSize(size_inv) + size_inv + witnessScript.GetVarSize(); | ||
networkFee += exec_fee_factor * MultiSignatureContractCost(m, n); | ||
} | ||
// We can support more contract types in the future. | ||
} | ||
else if (IsMultiSigContract(witnessScript, out int m, out int n)) | ||
networkFee += size * NativeContract.Policy.GetFeePerByte(snapshot); | ||
foreach (TransactionAttribute attr in tx.Attributes) | ||
{ | ||
int size_inv = 66 * m; | ||
size += IO.Helper.GetVarSize(size_inv) + size_inv + witnessScript.GetVarSize(); | ||
networkFee += exec_fee_factor * MultiSignatureContractCost(m, n); | ||
networkFee += attr.CalculateNetworkFee(snapshot, tx); | ||
} | ||
// We can support more contract types in the future. | ||
return networkFee; | ||
} | ||
networkFee += size * NativeContract.Policy.GetFeePerByte(snapshot); | ||
foreach (TransactionAttribute attr in tx.Attributes) | ||
catch (Exception ex) when (ex is not WalletException) | ||
{ | ||
networkFee += attr.CalculateNetworkFee(snapshot, tx); | ||
throw new WalletException(WalletErrorType.UnknownError, ex.Message, ex); | ||
} | ||
return networkFee; | ||
} | ||
|
||
internal static void ThrowIfNull(object argument, string paramName) | ||
{ | ||
if (argument == null) | ||
throw new WalletException(WalletErrorType.ArgumentNull, $"Argument {paramName} cannot be null."); | ||
} | ||
} | ||
} |
This file contains 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.
WalletException
constructor is able to createWalletException
fromWalletException
, so we can safely removewhen (ex is not WalletException)
.