Interop syscall for current tx signers

[shargon]

Close #2825

[shargon]

@roman-khimov please take a look to this version

[erikzhang]

I prefer to add a new syscall.

[shargon]

I prefer to add a new syscall.

GetTransactionSigners was extended, create a new one only for the current tx, seems weird to me :S

[shargon]

@neo-project/core what do you think about this change? now is not possible to check the signers of the current transaction, and this could be important for certain cases

[Jim8y]

@neo-project/core what do you think about this change? now is not possible to check the signers of the current transaction, and this could be important for certain cases

I am with @erikzhang on this. Maybe we should give it another syscall, cause current transaction may not actually be a part of the ledger.

[vncoelho]

Maybe one advantage of a new syscall is also to avoid problems during resync. @shargon , is this 100% compatible? Maybe state could change,no?

[Jim8y]

Maybe one advantage of a new syscall is also to avoid problems during resync. @shargon , is this 100% compatible? Maybe state could change,no?

A good question, if they called this on current tx before, this will change the state.

[superboyiii]

@shargon I think add a new one is better to avoid data incompatible.

[AnnaShaleva]

Just FYI, I've checked the compatibility with current mainnet up to 3722380 height using this patch applied to 0.101.2 (latest 3.5.0-compatible release) with NeoGo node, it doesn't produce a fork:

diff --git a/pkg/core/native/ledger.go b/pkg/core/native/ledger.go
index 51734c226..9b1f9bddc 100644
--- a/pkg/core/native/ledger.go
+++ b/pkg/core/native/ledger.go
@@ -154,6 +154,13 @@ func (l *Ledger) getTransactionFromBlock(ic *interop.Context, params []stackitem
 
 // getTransactionSigners returns transaction signers to the SC.
 func (l *Ledger) getTransactionSigners(ic *interop.Context, params []stackitem.Item) stackitem.Item {
+       hash, err := getUint256FromItem(params[0])
+       if err != nil {
+               panic(err)
+       }
+       if tx, ok := ic.Container.(*transaction.Transaction); ok && tx.Hash().Equals(hash) {
+               return SignersToStackItem(tx.Signers)
+       }
        tx, h, err := getTransactionAndHeight(ic.DAO, params[0])
        if err != nil || !isTraceableBlock(ic, h) {
                return stackitem.Null{}

Synced NeoGo node log:

...
2023-07-06T13:14:41.522+0300	INFO	node reached synchronized state, starting services
2023-07-06T13:14:41.522+0300	INFO	starting state validation service
2023-07-06T13:14:41.522+0300	INFO	RPC server already started
2023-07-06T13:14:43.654+0300	INFO	persisted to disk	{"blocks": 0, "keys": 2, "headerHeight": 3722380, "blockHeight": 3722380, "took": "2.419533ms"}
2023-07-06T13:14:55.668+0300	INFO	persisted to disk	{"blocks": 1, "keys": 24, "headerHeight": 3722381, "blockHeight": 3722381, "took": "9.178084ms"}
2023-07-06T13:14:59.665+0300	INFO	persisted to disk	{"blocks": 0, "keys": 2, "headerHeight": 3722381, "blockHeight": 3722381, "took": "4.620791ms"}
...

States check result:

anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ go run scripts/compare-states/compare-states.go http://localhost:10332 http://seed1.neo.org:10332
at 0: bb2bd2dfee742024b966cdf757c6235044cb9e0ce43497ad0eb27c49b8bdf4fe vs bb2bd2dfee742024b966cdf757c6235044cb9e0ce43497ad0eb27c49b8bdf4fe
at 3722380: ec321406a353f5906a049ebb034b72b8aeaeb7e07658d1bf6a8e84c0a7ba20ad vs ec321406a353f5906a049ebb034b72b8aeaeb7e07658d1bf6a8e84c0a7ba20ad

Although I'm not sure about T5 testnet.

[AnnaShaleva]

I've checked compatibility of this patch with T5 testnet as far, it's compatible up to 2306990:

...
2023-07-06T13:44:37.606+0300	INFO	node reached synchronized state, starting services
2023-07-06T13:44:37.606+0300	INFO	RPC server already started
2023-07-06T13:44:37.606+0300	INFO	starting state validation service
2023-07-06T13:44:53.741+0300	WARN	peer disconnected	{"addr": "65.108.90.74:21333", "error": "handling CMDExtensible message: invalid height", "peerCount": 12}
2023-07-06T13:45:18.242+0300	INFO	persisted to disk	{"blocks": 1, "keys": 23, "headerHeight": 2297514, "blockHeight": 2297514, "took": "1m31.495449456s"}
2023-07-06T13:45:24.609+0300	INFO	persisted to disk	{"blocks": 9474, "keys": 246120, "headerHeight": 2306988, "blockHeight": 2306988, "took": "6.366979915s"}
2023-07-06T13:45:25.617+0300	INFO	persisted to disk	{"blocks": 1, "keys": 23, "headerHeight": 2306989, "blockHeight": 2306989, "took": "5.461234ms"}
2023-07-06T13:45:26.619+0300	INFO	persisted to disk	{"blocks": 0, "keys": 2, "headerHeight": 2306989, "blockHeight": 2306989, "took": "7.163001ms"}
2023-07-06T13:45:38.647+0300	INFO	persisted to disk	{"blocks": 1, "keys": 23, "headerHeight": 2306990, "blockHeight": 2306990, "took": "27.762326ms"}
2023-07-06T13:45:43.625+0300	INFO	persisted to disk	{"blocks": 0, "keys": 2, "headerHeight": 2306990, "blockHeight": 2306990, "took": "2.693429ms"}
2023-07-06T13:45:53.633+0300	INFO	persisted to disk	{"blocks": 1, "keys": 23, "headerHeight": 2306991, "blockHeight": 2306991, "took": "5.431817ms"}
...

The states comparison result:

anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ go run scripts/compare-states/compare-states.go http://localhost:20332 http://seed1t5.neo.org:20332
at 0: 9c1361fbbe2a0ce31910f992cbdfb8afee6f178e5d1aabdd1c6f2af90440e239 vs 9c1361fbbe2a0ce31910f992cbdfb8afee6f178e5d1aabdd1c6f2af90440e239
at 2306990: f40800cc2da50ce3934795711d07d1bedf01ea91ab6de487cfad155a7f09cc89 vs f40800cc2da50ce3934795711d07d1bedf01ea91ab6de487cfad155a7f09cc89

So technically if nothinig breaking happens after the checked heights, this PR can be merged as is (without new interop).

However, we have #2873 that makes 3.6 incompatible with T5.

[superboyiii]

@vncoelho Could you have a review?

[vncoelho]

Sure, I will review asap

[superboyiii]

@shargon We need modify signer.cs

Otherwise will throw null exception

[shargon]

@shargon We need modify signer.cs Otherwise will throw null exception

Could you send me the error?

[Jim8y]

@superboyiii @shargon Lets move on to finish this

[shargon]

@superboyiii @shargon Lets move on to finish this

I'm waiting for @superboyiii , I didn't have this error

[ProDog]

When make the transaction, we need to calculate GasConsumed with InvokeScript in RpcClient, and AllowedContracts/AllowedGroups/Rules is null in RpcServer, so it will throw null exception.

https://github.com/neo-project/neo-modules/blob/09c2879958a916e0867fc78c64a04edfabe6935f/src/RpcServer/RpcServer.SmartContract.cs#L172-L174

Need to fix the RpcServer!

[superboyiii]

Good to merge.