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

Wallet.CalculateNetworkFee calculating incorrect NetworkFee #2388

Closed
devhawk opened this issue Mar 10, 2021 · 9 comments
Closed

Wallet.CalculateNetworkFee calculating incorrect NetworkFee #2388

devhawk opened this issue Mar 10, 2021 · 9 comments

Comments

@devhawk
Copy link
Contributor

devhawk commented Mar 10, 2021

Attempting to deploy contract via neo-express is failing validation due to insufficient NetworkFee. Wallet.CalculateNetworkFee appears to be calculating the network fee incorrectly. In the scenario below, the CalculateNetworkFee calculates the fee at 1627000 but verification fails because network fee should be 1736000

This seems like an important fix for RC1 #2329

var tx = wallet.MakeTransaction(neoSystem.StoreView, script, devAccount.ScriptHash, new[] { signer });
var networkFee = devWallet.CalculateNetworkFee(neoSystem.StoreView, tx); // network fee == 1627000
var context = new ContractParametersContext(neoSystem.StoreView, tx);
wallet.Sign(context);
tx.Witnesses = context.GetWitnesses();
var size = tx.Size; // size == 1736
networkFee = devWallet.CalculateNetworkFee(neoSystem.StoreView, tx);  // network fee == 1627000
var verificationContext = new TransactionVerificationContext();
var result = tx.Verify(ProtocolSettings, neoSystem.StoreView, verificationContext); // result == Insufficient Funds
@devhawk devhawk mentioned this issue Mar 10, 2021
27 tasks
@erikzhang
Copy link
Member

@neo-project/ngd-shanghai Please check it.

@cloud8little
Copy link
Contributor

Investigating.

@shargon
Copy link
Member

shargon commented Mar 11, 2021

@devhawk which kind of verification script?

@superboyiii
Copy link
Member

@devhawk What's the relationship between wallet and devwallet? I guess it's a signer difference which cause the netfee mismatch.

@cloud8little
Copy link
Contributor

@devhawk Could you give more detail on how this transaction looks like?
Not sure whether we have the same scenario, are you trying to deploy a contract with the account who have not enough gas and want to have another wallet which have enough gas as sender?
I tried with two wallets, devWallet(one account with 11 GAS) and CurrentWallet(2 account with 8.4622785/10.8772326).

CurrentWallet

NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j
NEO: 0
GAS: 8.4622785

NeGfNoazya5E6gM6DBahiRnhnAn3sMrYRV
NEO: 0
GAS: 10.8772326

devWallet

NKzjMjCfsE8C9Pb1YM4hHeDVUuDQFf1sPb
NEO: 0
GAS: 11
  1. CalculateNetworkFee has strong relationship with the calling wallet. because it will check whether the signer account belongs to the wallet. I guess that is why you got the same result.
  2. when tx do verify, its networkfee does not change before wallet.Sign(context) and after.

Scenario 1: sender and signer belongs to different wallet.

open wallet CurrentWallet, deploy contract with sender=NeGfNoazya5E6gM6DBahiRnhnAn3sMrYRV signer=NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j.

deploy TestHash160.nef TestHash160.manifest.json NKzjMjCfsE8C9Pb1YM4hHeDVUuDQFf1sPb NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j

neo> deploy TestHash160.nef TestHash160.manifest.json NKzjMjCfsE8C9Pb1YM4hHeDVUuDQFf1sPb NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j
1st:2041520
1st: Tx.Networkfee 2041520
{"invocation":"DEBGVitQIS\u002Bc3A/0uHdgWH1jEa4BNPXbBD/cz6RwBjemnctKRk0HUMG7tgyVLPBKgVzFRl7SMG2SvuHLITymXwXV","verification":"DCEDH6m11TSuyJ2V5wQg5zqustKm/HsRVlzxyNDYO6OumhNBdHR2qg=="}

{"invocation":"DEDItZzUBaBl\u002BXdYb4N6Znf/8oFMimmZyVYUwa0m65FNxBgezhZ9TtAx3PZJTxXuLuHvC1xdzLtCAne4TDUKrR/t","verification":"DCEDJNxhd91abS27VaXQ3Bvm\u002BL\u002BKHiyj3zJSgfXept9YVnpBdHR2qg=="}

witness varsize: 217
size: 1166
2nd:3133040
tx Netfee: 2041520
result:InsufficientFunds

Scenario 2: sender and signer belongs to the same wallet.

open wallet CurrentWallet, deploy contract with sender=NeGfNoazya5E6gM6DBahiRnhnAn3sMrYRV signer=NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j.

neo> deploy TestHash160.nef TestHash160.manifest.json NeGfNoazya5E6gM6DBahiRnhnAn3sMrYRV NT2EJfdipEc5ifwksZH2QynVHgShmV7b2j
1st:950000
1st: Tx.Networkfee 3133040
{"invocation":"DEAPeg1x2WuWw/PCrp1s8D9LwtmSdZZtrxB9LL2w/jzG1lJPfYFzvDKjf8DYH\u002BcOy0sKallVFKuj3c2vr3\u002BGbXTf","verification":"DCEDdZskjbzqXguci5XUPwYlo46yTLS9QXRVq1ux0H22d\u002BBBdHR2qg=="}

{"invocation":"DEDwqJ84\u002Bei/0GXCduN8\u002Bzxz6o/0am/GjgO4z\u002B8iPS4J\u002BJbWxoB6pEyHTmaRzHyy\u002Bb\u002BSFHEhGD\u002Bi3iVRNqP10Z9u","verification":"DCEDJNxhd91abS27VaXQ3Bvm\u002BL\u002BKHiyj3zJSgfXept9YVnpBdHR2qg=="}

witness varsize: 217
size: 1166
2nd:3133040
tx Netfee: 3133040
result:Succeed

@devhawk
Copy link
Contributor Author

devhawk commented Mar 11, 2021

@devhawk What's the relationship between wallet and devwallet? I guess it's a signer difference which cause the netfee mismatch.

@superboyiii : ExpressWallet/ExpressWalletAccount are simple models types for JSON serialization.

DevWallet/DevWalletAccount are subclasses of Wallet/WalletAccount for use in Neo Express - for example, DevWallet/Account don't password protect private keys.

There is no signing code in DevWallet that isn't inherited from Wallet

@devhawk
Copy link
Contributor Author

devhawk commented Mar 11, 2021

@devhawk which kind of verification script?

@shargon whatever ContractParametersContext.GetWitnesses creates

@devhawk
Copy link
Contributor Author

devhawk commented Mar 11, 2021

Note, I think the problem is that CreateMultiSigRedeemScript and CreateSignatureRedeemScript have changed since preview 5, so my old neo express files are invalid

@ZhangTao1596
Copy link
Contributor

Note, I think the problem is that CreateMultiSigRedeemScript and CreateSignatureRedeemScript have changed since preview 5, so my old neo express files are invalid

Close?

@devhawk devhawk closed this as completed Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants