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

Can not calculate the network fee when trying to multi-sign a transaction #3429

Closed
luc10921 opened this issue May 2, 2024 · 5 comments
Closed
Labels
bug Something isn't working I4 No visible changes S4 Routine U2 Seriously planned

Comments

@luc10921
Copy link

luc10921 commented May 2, 2024

Current Behavior

The following error is thrown: Encountered HTTP code 422 while executing Query[calculatenetworkfee].
When running it locally, the log is: Error encountered with rpc request {"code": -508, "cause": "witness hash mismatch", "method": "calculatenetworkfee", "params": "[AFGE2pUAAAAAAAAAAAAAAAAAAAAAawAAAAINqWekAEMr8n+OjrRv6KxlnszeBAFt/ZapzGBDT9YILYp1q1ufMj9X9AEAXAwEdGVzdABkDBQNqWekAEMr8n+OjrRv6KxlnszeBAwUbf2WqcxgQ0/WCC2KdatbnzI/V/QUwB8MCHRyYW5zZmVyDBTPduKL0AYsSkeO41VhARMZ88+k0kFifVtSAkIMQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoDCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJ0IMQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoDCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJw==]"}

Expected Behavior

This multi-sign has been tested multiple times against Neo Nodes on the TestNet and should be working. However, we encountered an issue when we decided to use neo-go locally for the unit tests. We also tested it against https://rpc.t5.n3.nspcc.ru:20331/, where it similarly failed to function.

Possible Solution

It seems like Neo's node doesn't check the witnesses on the transaction when doing the calculatenetworkfee.

Steps to Reproduce

You can do a request using the following body:

{
  "jsonrpc": "2.0",
  "method": "calculatenetworkfee",
  "params": ["AFGE2pUAAAAAAAAAAAAAAAAAAAAAawAAAAINqWekAEMr8n+OjrRv6KxlnszeBAFt/ZapzGBDT9YILYp1q1ufMj9X9AEAXAwEdGVzdABkDBQNqWekAEMr8n+OjrRv6KxlnszeBAwUbf2WqcxgQ0/WCC2KdatbnzI/V/QUwB8MCHRyYW5zZmVyDBTPduKL0AYsSkeO41VhARMZ88+k0kFifVtSAkIMQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoDCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJ0IMQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAoDCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJw=="],
  "id": 1
}

If you send it to http://seed1t5.neo.org:20332, you'll get a value, but if you try to send it to https://rpc.t5.n3.nspcc.ru:20331, it will be the code: -508.

Alternatively, you can view and clone the unit tests that target the Neo's TestNet here.
Instead of using rpcAddress: NeonInvoker.TESTNET, we used https://rpc.t5.n3.nspcc.ru:20331/ and it also failed.

Context

We are developing a feature in NeonDappkit and WalletConnectSDK that enables dApps to sign transactions and then prompts users to sign them as well.

@luc10921 luc10921 added bug Something isn't working U2 Seriously planned labels May 2, 2024
@roman-khimov roman-khimov added S4 Routine I4 No visible changes labels May 2, 2024
@roman-khimov
Copy link
Member

Your transaction is:

{
      "Version": 0,
      "Nonce": 2514125905,
      "SystemFee": 0,
      "NetworkFee": 0,
      "ValidUntilBlock": 107,
      "Script": "DAR0ZXN0AGQMFA2pZ6QAQyvyf46OtG/orGWezN4EDBRt/ZapzGBDT9YILYp1q1ufMj9X9BTAHwwIdHJhbnNmZXIMFM924ovQBixKR47jVWEBExnzz6TSQWJ9W1I=",
      "Attributes": [],
      "Signers": [
           {
                "account": "0x04decc9e65ace86fb48e8e7ff22b4300a467a90d",
                "scopes": "CalledByEntry"
           },
           {
                "account": "0xf4573f329f5bab758a2d08d64f4360cca996fd6d",
                "scopes": "CalledByEntry"
           }
      ],
      "Scripts": [
           {
                "invocation": "DEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
                "verification": "DCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJw=="
           },
           {
                "invocation": "DEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
                "verification": "DCEDWpKPIBY5IE4GtDaLGpM2VGKo67/wuIGBUbdPqrOithpBVuezJw=="
           }
      ],
      "Trimmed": false
 }

So you have two signers here. Then you pass two witnesses with this verification code:

INDEX    OPCODE       PARAMETER
0        PUSHDATA1    035a928f201639204e06b4368b1a93365462a8ebbff0b8818151b74faab3a2b61a    <<
35       SYSCALL      System.Crypto.CheckSig (56e7b327)

Which hashes down to 0x04decc9e65ace86fb48e8e7ff22b4300a467a90d. This is the hash of the first signer, but the second one is different, that's why you get an error:

{"id":1,"jsonrpc":"2.0","error":{"code":-508,"message":"Invalid signature","data":"witness hash mismatch"}}

And it's a valid one, if you're to take the network fee data, stuff it into your transaction, sign and try to send it --- it'd be invalid. To make it a valid one you need a proper script for the second signer, if you're to provide that NeoGo will happily calculate the network fee. Right now it can't, it doesn't have a script for 0xf4573f329f5bab758a2d08d64f4360cca996fd6d signer (which btw, can be anything, not just a simple signature, but we don't know).

NOBUG.

@roman-khimov roman-khimov closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2024
@lock9
Copy link

lock9 commented May 14, 2024

Hi @roman-khimov,

We don't have access to the verification script, so we are using a placeholder. To produce a valid verification script, we would need the public key. It looks like C# parses it as a 'placeholder'. I understand that this is not ideal: it always calculates the fees using a 'single-sig account,' whereas, in practice, it could be a multi-sig.

The main issue is that the application works with C# nodes but not with Neo Go nodes. This inconsistency causes it to work for some users and not for others. If the signer is not present, could it be considered a default single-signature account? (mimic c#)

@roman-khimov
Copy link
Member

We don't have access to the verification script, so we are using a placeholder

But you have to get a proper verification script and put it into transaction. You just have to do this before calculatenetworkfee and not after that. Otherwise you can stub the signer account as well (use a random key and account), it'll work fine for fee calculation.

The main issue is that the application works with C# nodes but not with Neo Go nodes.

neo-project/neo#2805 will be implemented eventually and I expect C# nodes to get this behavior as well.

@lock9
Copy link

lock9 commented May 14, 2024

Hello,
Thank you for the super fast response.

I need to use the actual signers; otherwise, the check witness calls will fail. If it fails, the system fee will be incorrect.

EDIT: I get it now. We can use the stub to calculate the network fee. It won't affect the system fee because it's calculated using a test invoke

@roman-khimov
Copy link
Member

I need to use the actual signers; otherwise, the check witness calls will fail. If it fails, the system fee will be incorrect.

These are different fees and nothing prevents you from using one set of signers for system fee calculation and the other for network fee calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working I4 No visible changes S4 Routine U2 Seriously planned
Projects
None yet
Development

No branches or pull requests

3 participants