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

[Question] Does CalculateNetworkFee(DataCache snapshot, Transaction tx) needs to be inside Wallet? Make network fee calculation more generic. #2352

Closed
vncoelho opened this issue Feb 18, 2021 · 2 comments · Fixed by #3147
Labels
question Used in questions

Comments

@vncoelho
Copy link
Member

The call

public long CalculateNetworkFee(DataCache snapshot, Transaction tx)
{
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);
uint exec_fee_factor = NativeContract.Policy.GetExecFeeFactor(snapshot);
long networkFee = 0;
foreach (UInt160 hash in hashes)
{
byte[] witness_script = GetAccount(hash)?.Contract?.Script;
if (witness_script is null && tx.Witnesses != null)
{
// Try to find the script in the witnesses
foreach (var witness in tx.Witnesses)
{
if (witness.ScriptHash == hash)
{
witness_script = witness.VerificationScript;
break;
}
}
}
if (witness_script is null)
{
var contract = NativeContract.ContractManagement.GetContract(snapshot, hash);
if (contract is null) continue;
var md = contract.Manifest.Abi.GetMethod("verify", 0);
if (md is null)
throw new ArgumentException($"The smart contract {contract.Hash} haven't got verify method without arguments");
if (md.ReturnType != ContractParameterType.Boolean)
throw new ArgumentException("The verify method doesn't return boolean value.");
// Empty invocation and verification scripts
size += Array.Empty<byte>().GetVarSize() * 2;
// Check verify cost
using ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, tx, snapshot.CreateSnapshot(), settings: ProtocolSettings);
engine.LoadContract(contract, md, 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.");
networkFee += engine.GasConsumed;
}
else if (witness_script.IsSignatureContract())
{
size += 67 + witness_script.GetVarSize();
networkFee += exec_fee_factor * SignatureContractCost();
}
else if (witness_script.IsMultiSigContract(out int m, out int n))
{
int size_inv = 66 * m;
size += IO.Helper.GetVarSize(size_inv) + size_inv + witness_script.GetVarSize();
networkFee += exec_fee_factor * MultiSignatureContractCost(m, n);
}
else
{
//We can support more contract types in the future.
}
}
networkFee += size * NativeContract.Policy.GetFeePerByte(snapshot);
return networkFee;
}
is currently inside Wallet and this makes it requires a Dummy Wallet on neo-modules RpcServer https://github.com/neo-project/neo-modules/blob/ccc2ff93599c8127ad2d5a2a21f99360d84c79b3/src/RpcServer/RpcServer.Wallet.cs#L124.

What necessarily makes it need an open wallet to make this calculation?

On NEO 2 perhaps yes, due to UTXO calculations regarding size.
However, now, we mostly just use UInt160[] hashes = tx.GetScriptHashesForVerifying(snapshot); which are already created and then we get the VerificationScript using GetAccount(hash)?.Contract?.Script;

I suggested to restructure these functions to a different place on the neo-core and even move them from the Rpc-Wallet, making them more user friendly in a way that VerificationScripts can be passed as parameters for a generic calculation.

@vncoelho vncoelho added the question Used in questions label Feb 18, 2021
@Qiao-Jin
Copy link
Contributor

I think we still need some features in Wallet such as func GetAccount for this calculation

@erikzhang
Copy link
Member

I think we still need some features in Wallet such as func GetAccount for this calculation

Agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Used in questions
Projects
None yet
3 participants