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

Make Witness Scripts available to smart contract APIs #3794

Closed
lock9 opened this issue Feb 26, 2025 · 21 comments
Closed

Make Witness Scripts available to smart contract APIs #3794

lock9 opened this issue Feb 26, 2025 · 21 comments
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@lock9
Copy link
Contributor

lock9 commented Feb 26, 2025

Summary or problem description
We send additional parameters in the witness invocation script. This parameter is important to enhance our project security. Since saving this value during the verification phase is impossible, one alternative would be to read and decode the witness scripts.

Do you have any solution you want to propose?
Add a new interop that allows contracts to retrieve contract witnesses' scripts.

Where in the software does this update apply to?

  • Compiler
  • VM
@lock9 lock9 added the Discussion Initial issue state - proposed but not yet accepted label Feb 26, 2025
@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 26, 2025

All you need is this the public key and you can create it. It very easy.

var sb = new ScriptBuilder().Push(tx.Sign(kp.PrivateKey, settings.Network)).ToArray(); // verification script.

@shargon
Copy link
Member

shargon commented Feb 27, 2025

It could be not deterministic, so it's not possible

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 27, 2025

You can already do Interop in ApplicationEngine just have to create your own like @Hecate2 did for this project. If its for a contract on blockchain. Than you add verify method to contract with your parameters.

@lock9
Copy link
Contributor Author

lock9 commented Feb 27, 2025

It could be not deterministic, so it's not possible

What do you mean? I don't mean the execution outcome, I mean the script itself, the bytes.

@cschuchardt88 I do not understand your comments

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 27, 2025

@lock9 you can use verify method of your contract to to verify your security of the parameter.

You can use NEP-30. If you need an example let me know.

@lock9
Copy link
Contributor Author

lock9 commented Feb 27, 2025

But I am using the verify method. The problem is that it can't change the storage (afaik) when it's called. However, the witness verification and invocation scripts are saved. The 'trick' that I'm trying to do is to send an extra parameter in the invocation script. Then, I decode the script to obtain it.

Something like this:

func MyMethod(txHash hash160, counter int){
   witnessScript = runtime.getWitnessScripts(txHash, 0)
   decodedScript = utils.deccodeMyCustomVerificationScript(witnessScript.verificationScript, witinessScript.invocationScript)
   if decodedScript.authenticationCounter > counter:
       return false

   return true
}

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 27, 2025

For Contract on-chain do:

If you want storage in verify method just dont use when TriggerType.Verification and can't have [safe] attribute on method. Follow example.

Example

// add more parameters if needed
public static bool Verify(object value)
{
    bool result = false;

    if (Runtime.Trigger == TriggerType.Verification)
    {
        // do verification without save.
    }
    else
    {
        // do verification with save.
        // Storage.Put(key, value);
    }

    return result;
}

In transaction

var ownerContract = Contract.CreateSignatureContract(keyPair.PublicKey);
var myContractHash = UInt160.Parse("<Contract ScriptHash Here>");

using var txScript = new ScriptBuilder()
    .EmitDynamicCall(myContractHash, "verify", myParameterValue/* Add more parameters here for your verify method if needed *//);

Transaction tx = new Transaction
{
    Version = 0,
    Nonce = 0,                  // Doesn't matter what this is.
    SystemFee = 0_00000000,     // Calculate your system fee
    NetworkFee = 0_00000000,    // Calculate your network fee
    ValidUntilBlock = NativeContract.Ledger.CurrentIndex(snapshot) + 1000,
    Script = txScript.ToArray(),
    Attributes = [],
    Signers = [
        new Signer() { Account = myContractHash, Scopes = WitnessScope.CalledByEntry },
        new Signer() { Account = ownerContract.ScriptHash, Scopes = WitnessScope.CalledByEntry },
    ],
    Witnesses = [],
};

using var ownerContractInvocationScript = new ScriptBuilder()
    .EmitPush(tx.Sign(keyPair.PrivateKey, protocolSettings.Network));

// If you need more parameters add them here with `EmitPush`
using var myContractVerifyParameterInvocationScript = new ScriptBuilder()
    .EmitPush(myParameterValue);

tx.Witnesses = [
    new Witness { InvocationScript = myContractVerifyParameterInvocationScript.ToArray(), VerificationScript = null },
    new Witness { InvocationScript = ownerContractInvocationScript.ToArray(), VerificationScript = ownerContract.Script },
];

var sendRawTransaction = Convert.ToBase64String(tx.ToArray());

Console.WriteLine("Raw Transaction: {0}", sendRawTransaction);

@lock9
Copy link
Contributor Author

lock9 commented Feb 27, 2025

I don't know if I understand what you are proposing. I can't change the user transaction and the trigger type will be "verification". It must work with any transaction, without changes to the transaction script.

@Hecate2
Copy link
Contributor

Hecate2 commented Feb 28, 2025

Do you mean that you want a single public key to pass your verification for only a limited count of times?

@lock9
Copy link
Contributor Author

lock9 commented Feb 28, 2025

Do you mean that you want a single public key to pass your verification for only a limited count of times?

Sort of. I wish to check if a 'counter' is not below or equal to a certain value. I don't control it, it's incremented automatically by a third party. This value won't be included in the transaction script but in the witness scripts.

However:

  • The most secure solution would be always to store it when it passes verification.
  • Not all accounts increment this value, so sometimes this verification will be skipped

In theory, you can't save anything during the witness script execution, however, the script witnesses are stored. I want to use this 'loophole' to persist this information, without using the storage APIs.

@Hecate2
Copy link
Contributor

Hecate2 commented Feb 28, 2025

In theory, you can't save anything during the witness script execution, however, the script witnesses are stored. I want to use this 'loophole' to persist this information, without using the storage APIs.

I really wonder why not store something (like scriptHash -> invocationCount) in the business contract when CheckWitness is called. If the third party is unable to determine whether the verification count should increment, you can use a pessimistic design. More verifications always require an approval from the third party.

@lock9
Copy link
Contributor Author

lock9 commented Feb 28, 2025

I really wonder why not store something (like scriptHash -> invocationCount) in the business contract when CheckWitness is called. If the third party is unable to determine whether the verification count should increment, you can use a pessimistic design. More verifications always require an approval from the third party.

The software I'm working on is a special type of wallet. It's responsible for signing transactions (generating correct witnesses scripts). We use a smart contract, but it's called in the witness script, not on the transaction script.

All the transactions are created by the dapps, which then use dAPI to interact with the wallet. We can't/shouldn't change it. We can manipulate the witness scripts because we are 'the wallet', but not the transaction script itself.

Note: I want to check this value from a previous transaction, not in the transaction being executed.

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 28, 2025

You could of used Runtime.LoadScript(tx.Witnesses[0].InvocationScript, CallFlags.All). However, when neo converts the transaction to interop stackitem it does not add all the properties of the transaction, like witnesses. Maybe @shargon can answer that question. Why the full transaction isnt available.

@shargon
Copy link
Member

shargon commented Feb 28, 2025

Imagine a multisig 1/5, the same transaction can be relayed and be valid in different nodes, but with different signature, so it's not deterministic, that's the reason why the witnesses are not available in the smart contract

@roman-khimov
Copy link
Contributor

require an approval from the third party

Welcome to #2577. The same solution basically.

@lock9
Copy link
Contributor Author

lock9 commented Feb 28, 2025

Imagine a multisig 1/5, the same transaction can be relayed and be valid in different nodes, but with different signature, so it's not deterministic, that's the reason why the witnesses are not available in the smart contract

Even when it's a finalized transaction? Because we can retrieve this information using RPC.

Welcome to #2577. The same solution basically.

I just checked this issue. We were able to implement this using contracts. However, there's one thing there that got me worried:

it can't differentiate between None and Global scopes for Sender which is very dangerous.

Maybe I misunderstood this part because we can get the signer scopes. Is this a new feature?

Is there anything wrong with the code below? It seems to work fine

func Verify() bool {
	signers := runtime.CurrentSigners()
	transactionSponsor := signers[0]
	txScript := runtime.GetScriptContainer().Script

	isGateKeeperCall := IsGateKeeperCall(txScript)

	if !isGateKeeperCall {
		panic("Not gatekeeper call")
	}

	if transactionSponsor.Scopes != 0x00 {
		panic("Invalid scope. Expected None, received " + std.Itoa(int(transactionSponsor.Scopes), 16))
	}

	if std.MemoryCompare(transactionSponsor.Account, runtime.GetExecutingScriptHash()) != 0 {
		panic("Invalid account")
	}

	return true
}

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 28, 2025

Imagine a multisig 1/5, the same transaction can be relayed and be valid in different nodes, but with different signature, so it's not deterministic, that's the reason why the witnesses are not available in the smart contract

How is that? If ScriptContainer is an transaction all its data is there. It's just not adding in ToStackItem for system call GetScriptContainer. One big reason why I don't believe this is because the way GetRandom works. GetRandom adds a number that is incremented from memory, so if you restart the node it will start back at zero compared to a node that is still running. and no other consensus node will produce the same result. So why does that work? If what you say is true.

@roman-khimov
Copy link
Contributor

roman-khimov commented Feb 28, 2025

Even when it's a finalized transaction?

Yes. Just think of it in a bit another way --- invocation script is a script, the outcome of execution (invocation+verification) defines the result. You can add a NOP to any script without changing this outcome. Or two. Or three. Or a PUSH/POP pair Or... All of these scripts would be valid yet they all will be different.

Because we can retrieve this information using RPC.

Try fetching current mainnet blocks from various RPCs, they're 5/7 multisig by nature and you have a pretty good probability of catching this in block witnesses.

we can get the signer scopes. Is this a new feature?

Relatively. #2577 is Aug 2021, it mentions #2460 (May 2021) that was fixed in #2685 (May 2022). So this part of the problem is no longer relevant, you're using runtime.CurrentSigners() correctly.

@lock9
Copy link
Contributor Author

lock9 commented Feb 28, 2025

Try fetching current mainnet blocks from various RPCs, they're 5/7 multisig by nature and you have a pretty good probability of catching this in block witnesses.

Does this mean that we have the same block with different witnesses?

@roman-khimov
Copy link
Contributor

Yes. And all of these witnesses are valid.

@lock9
Copy link
Contributor Author

lock9 commented Feb 28, 2025

Alright. I get it now. It makes sense. I thought that the CN nodes would eventually agree on just one 'witness state', but that's not true. Not doable - case closed.

Thanks a lot for the responses.

@lock9 lock9 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

No branches or pull requests

5 participants