neo icon indicating copy to clipboard operation
neo copied to clipboard

Ledger.GetTransaction() can't get current transaction

Open RichardBelsum opened this issue 4 years ago • 26 comments

I used method Ledger.GetTransaction() in the smart contract, and I expect it to return true when pre-execution, and false when I actually execution it.

Contract code

public static bool t1()
{
    return Ledger.GetTransaction(((Transaction)Runtime.ScriptContainer).Hash) == null;
}

Invokefunction(pre-execution) request

{
    "jsonrpc": "2.0",
    "id": 1,
    "method": "invokefunction",
    "params": [
        "0xd2448d18052d5732f7da35d5d31e6ef47e664508",
        "t1",
        [],
        [
            {
                "account": "0x4578060c29f4c03f1e16c84312429d991952c94c",
                "scopes": "CalledByEntry",
                "allowedcontracts": [],
                "allowedgroups": []
            }
        ]
    ]
}

response

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
        "script": "wh8MAnQxDBQIRWZ+9G4e09U12vcyVy0FGI1E0kFifVtS",
        "state": "HALT",
        "gasconsumed": "4718760",
        "exception": null,
        "stack": [
            {
                "type": "Boolean",
                "value": true
            }
        ],
        "tx": "ANaI8RGoAEgAAAAAAPjrEQAAAAAAlhoAAAFMyVIZmZ1CEkPIFh4/wPQpDAZ4RQEAIcIfDAJ0MQwUCEVmfvRuHtPVNdr3MlctBRiNRNJBYn1bUgFCDEBB6QBbnLMUY/Vw4MDHdRClOeYy02quh/S3f7PNq5pOud8EkCWIVq+4w39RRbbr5wCpWGhc5IiG2eQRSHFANngqKAwhAnuOhIl9zf58vffm17ad227dYyPL5zLqaN0rPUSoDoDwQVbnsyc="
    }
}

Then I sent the transaction and got the transaction log: (actually execution)

{
    "jsonrpc": "2.0",
    "id": 1,
    "result": {
        "txid": "0x5490875a4a161f457de2de8553af13b089fd36e8092b7fe199d85c13651f5ea0",
        "executions": [
            {
                "trigger": "Application",
                "vmstate": "HALT",
                "exception": null,
                "gasconsumed": "4718760",
                "stack": [
                    {
                        "type": "Boolean",
                        "value": true
                    }
                ],
                "notifications": []
            }
        ]
    }
}

So, Ledger.GetTransaction can't get current transaction?

RichardBelsum avatar Aug 11 '21 07:08 RichardBelsum

I can get the transaction in the previous blocks, such as:

public static bool t2(UInt256 tx)
{
    return Ledger.GetTransaction(tx) == null;
}

image

RichardBelsum avatar Aug 11 '21 08:08 RichardBelsum

So, Ledger.GetTransaction can't get current transaction?

Yes. As well as any other transaction from the same block, they're not traceable. https://github.com/neo-project/neo/blob/a4d2eddbee049bbe3d5d9552e11181d294af929e/src/neo/SmartContract/Native/LedgerContract.cs#L54-L59 Which actually allows for some interesting optimizations.

roman-khimov avatar Aug 11 '21 08:08 roman-khimov

First, the modification of the Prefix_CurrentBlock value is done in the postPersist phase.

image

Then CurrentIndex actually gets the Index of the previous block image

Then, IsTraceableBlock return false. image

Then, GetTransactionForContract return null. image

RichardBelsum avatar Aug 11 '21 08:08 RichardBelsum

I think you can find the answer in your code, you were asking your transaction from the Ledger while your transaction was not in the Ledger, it was still persisting.

Jim8y avatar Aug 11 '21 08:08 Jim8y

So, it's by design, not bug?

RichardBelsum avatar Aug 11 '21 08:08 RichardBelsum

So, it's by design, not bug?

At least you should not expect to get persisting transactions in a Ledger. The Ledger only contains persisted transactions.

Jim8y avatar Aug 11 '21 08:08 Jim8y

I think you can find the answer in your code, you were asking your transaction from the Ledger while your transaction was not in the Ledger, it was still persisting.

Agree

shargon avatar Aug 12 '21 18:08 shargon

Transaction current_tx = (Transaction)Runtime.ScriptContainer;

erikzhang avatar Aug 15 '21 02:08 erikzhang

There may be some misunderstanding. I want to use GetTransaction() to determine whether the currently executing smart contract is a pre-execution or actually execution.

RichardBelsum avatar Aug 16 '21 02:08 RichardBelsum

Because the paths of pre-execution and actual execution may not match (1), and now I can't use Gas.Refuel. So I want to increase the pre-execution fee by determining whether the contract is pre-executed or not.

1: e.g.

var index = Storage.Get(Storage.CurrentContainer,"index");

if(index % 2 == 0)
{
    //Some of the codes that consume GAS
}

Storage.Put(Storage.CurrentContainer,"index", index + 1);

RichardBelsum avatar Aug 16 '21 02:08 RichardBelsum

Any updates? I think it's a bug, do you agree?

RichardBelsum avatar Aug 18 '21 07:08 RichardBelsum

Any updates? I think it's a bug, do you agree?

Why do you think it is a bug?

Jim8y avatar Aug 18 '21 07:08 Jim8y

Any updates? I think it's a bug, do you agree?

Why do you think it is a bug?

As long as the transaction is on the chain, it should be traceable

RichardBelsum avatar Aug 18 '21 07:08 RichardBelsum

Because the paths of pre-execution and actual execution may not match

The execution path of actual execution surely can be different from pre-execution and cannot be controlled, depending on balance, constrained condition within script logic, etc. I'm not clear what's its relationship to the feature you want.

Qiao-Jin avatar Aug 18 '21 07:08 Qiao-Jin

I want to determine in the contract whether the code is being pre-executed, and if so, add some code to increase the fee to avoid the actual execution fee being insufficient @Qiao-Jin

e.g.

if(Ledger.GetTransaction(((Transaction)Runtime.ScriptContainer).Hash) == null)
{
    //only pre-execution
    for(int i = 0; i < 10; i++)
        Storage.Put(Storage.CurrentContext, "temp", "abc");
}

var index = Storage.Get(Storage.CurrentContainer,"index");
if(index % 2 == 0)
{
    //Some of the codes that consume GAS
}
Storage.Put(Storage.CurrentContainer,"index", index + 1);

RichardBelsum avatar Aug 18 '21 07:08 RichardBelsum

  • a transaction in execution is not on the chain, on the chain means in the ledger
  • Qiao-Jin answers well.
  • contract does not handle fees.
  • pre-execution could consume (not really consumes) more gas, it could also consume (not really consumes) less gas, we can not know.

Jim8y avatar Aug 18 '21 07:08 Jim8y

You can add some GAS to system fee when you're creating a transaction, contract itself shouldn't even try to do estimations of this kind. And it shouldn't differentiate between on-chain and test executions in general, otherwise what's use of test execution? It's not just about GAS, I'd also like to see exactly what happens when I do something with the contract, if the contract decides to change its behavior on-chain I'd say this contract is cheating me.

roman-khimov avatar Aug 18 '21 07:08 roman-khimov

When you in pre-execution, you are always the first transaction, which is not the case when you actually execute. The person in front of you will modify the storage, causing you to have a different execution path and different fees.

I can write code to add systemfee, but normal users can only call the contract with a wallet(like NeoLine/O3), they don't support adding systemfee

RichardBelsum avatar Aug 18 '21 07:08 RichardBelsum

I could write it that way before, but now I need a new way to replace Gas.Refuel.

Gas.Refuel((Transaction)Runtime.ScriptContainer).Sender, 1000000);

var index = Storage.Get(Storage.CurrentContainer,"index");
if(index % 2 == 0)
{
    //Some of the codes that consume GAS
}
Storage.Put(Storage.CurrentContainer,"index", index + 1);

RichardBelsum avatar Aug 18 '21 07:08 RichardBelsum

When you in pre-execution, you are always the first transaction, which is not the case when you actually execute. The person in front of you will modify the storage, causing you to have a different execution path and different fees.

True, but

  • it's not often that it happens
  • in a system that we have it's solved by additional system fee that is being added by entity generating transaction

Think of it the other way around, if your contract always burns additional GAS (not needed for its logic) then users always pay this additional GAS for nothing. They might be upset by this too.

But there was a proposal to return unspent system fee to senders which could technically allow to always safely add some GAS (#1536), maybe that could be something to consider long-term. While for now we have what we have.

roman-khimov avatar Aug 18 '21 08:08 roman-khimov

Assuming that pre-execution requires 2GAS and the actual execution has a 20% chance of requiring 2.01GAS, I would rather add 0.01 GAS to each transaction, otherwise I would lose 2GAS because the contract execution would fail due to sufficient fees.

The wallet(Neo-cli/Neo3-GUI/NeoLine/O3/Neon) used by the user can't add systemfee, so I can only add systemfee to the pre-execution of the contract.

RichardBelsum avatar Aug 18 '21 09:08 RichardBelsum

Assuming that pre-execution requires 2GAS and the actual execution has a 20% chance of requiring 2.01GAS, I would rather add 0.01 GAS to each transaction, otherwise I would lose 2GAS because the contract execution would fail due to sufficient fees.

The wallet(Neo-cli/Neo3-GUI/NeoLine/O3/Neon) used by the user can't add systemfee, so I can only add systemfee to the pre-execution of the contract.

Then you can open an issue for that. I agree with you that wallet should allow users to pay more gas.

Jim8y avatar Aug 18 '21 09:08 Jim8y

Add a maxExecFee field for each method in ABI or manifest?

erikzhang avatar Aug 18 '21 09:08 erikzhang

Add a maxExecFee field for each method in ABI or manifest?

However, the committee can dynamically adjust the fee factor. If so the developer will have to update the contract after each fee change.

RichardBelsum avatar Aug 18 '21 09:08 RichardBelsum

Add a maxExecFee field for each method in ABI or manifest?

I doubt it's viable. What is the max cost of GAS transfer?

roman-khimov avatar Aug 18 '21 09:08 roman-khimov

Add a maxExecFee field for each method in ABI or manifest?

Such a function, no maxExecFee too.

public static bool Airdrop(BigInteger amount)
{
    for (int i = 0; i < amount; i++)
    {
        ……
        Mint(tokenId, tokenState);
    }
    return true;
}

RichardBelsum avatar Aug 18 '21 09:08 RichardBelsum