Older hook states are not getting deleted when state_set to 0

chalith commented 2 years ago

Issue Description

When state_set(0, 0, SBUF(key)) is used to delete an older state object, It looks deleted when the we check for the existence of that state object withing the same hook execution after the deletion. But if we check for that deleted state object in an another hook execution, it's still there. And the state value is also consistent with the original value (As it was not deleted at all).

Checked the same steps for a newly created hook state. But it's getting deleted without an issue and stays deleted for next executions as well.

Steps to Reproduce

This is my hook contract:

int64_t hook(int64_t reserved)
{
    // Older state, which is getting deleted in this execution, but it's there again in next execution.
    {
        uint8_t key_bytes[32] = {0x45, 0x56, 0x52, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7E, 0xA5, 0xA5, 0x3B,
                                 0x49, 0xF0, 0x06, 0x5B, 0x26, 0x0C, 0xB0, 0x00, 0xB2, 0xA0, 0x27, 0xF8, 0x72, 0x93, 0xD4, 0x37};
        // hexstr = 4556520300000000000000007ea5a53b49f0065b260cb000b2a027f87293d437
        trace(SBUF("OLDER STATE: KEY ================="), SBUF(key_bytes), 1);

        uint8_t existing_data[128];
        if (state(SBUF(existing_data), SBUF(key_bytes)) > 0) // Checking whether state exists.
        {
            trace(SBUF("OLDER STATE: EXISTS - DELETING ================="), SBUF(existing_data), 1);
            if (state_set(0, 0, SBUF(key_bytes)) == 0) // Delete the state.
            {
                trace(SBUF("OLDER STATE: DELETED ================="), 0, 0, 0);
                uint8_t check_data[128];
                // Checking whether still exists, this won't get printed since it's getting deletd in this execution.
                if (state(SBUF(check_data), SBUF(key_bytes)) > 0)
                    trace(SBUF("OLDER STATE: DELETED BUT STILL EXISTS ================="), SBUF(check_data), 1);
            }
        }
    }

    // New state, which is getting deleted without issue.
    {
        uint8_t key_bytes[32] = {0x45, 0x56, 0x52, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xA5, 0xC3, 0xA7, 0xA1,
                                 0x90, 0x0A, 0x5B, 0xCD, 0xAF, 0x25, 0xBA, 0x8A, 0x2E, 0xDE, 0x44, 0x07, 0x57, 0xFD, 0xB2, 0x71};
        // hexstr = 455652030000000000000000A5C3A7A1900A5BCDAF25BA8A2EDE440757FDB271
        trace(SBUF("NEW STATE: KEY ================="), SBUF(key_bytes), 1);

        uint8_t existing_data[128];
        if (state(SBUF(existing_data), SBUF(key_bytes)) > 0) // Checking whether state exists.
        {
            trace(SBUF("NEW STATE: STATE EXISTS - DELETING ================="), SBUF(existing_data), 1);
            if (state_set(0, 0, SBUF(key_bytes)) == 0) // Delete the state.
                trace(SBUF("NEW STATE: DELETED ================="), 0, 0, 0);
        }
        else // If state does not exist create and check whether it's actually deleted in next execution.
        {
            trace(SBUF("NEW STATE: NOT EXISTS - CREATING ================="), 0, 0, 0);
            if (state_set(SBUF("TEST DATA"), SBUF(key_bytes)) > 0) // Create the state.
                trace(SBUF("NEW STATE: CREATED ================="), 0, 0, 0);
        }
    }

    accept(SBUF("ACCEPTED ================="), 0);
    _g(1, 1);
    return 0;
}

Deploy this contract to following hook account in the testnet.

address - rpTA2MtFV7L6hLzjSBYc1FBgYqTs6APVdz secret - spAhiYQMb71CyEHU4zAA5Q2PSiUkN

Code explanation

The first block is where I try to delete the older state with it's key.

First, check the existence of that key and delete if it exists.

After deleting, check whether it's actually deleted.

The second block is where I try to delete a new state with it's key.

First, check the existence of that key, create one if not exists, otherwise delete.

Try issuing multiple transactions and check the logs for deletions in each execution (transaction).
In the code, trace logs are prefixed as OLDER STATE: for old state block and NEW STATE: for new state block.

Expected Result

For older state object, trace prefix: OLDER STATE:

1st execution (transaction) - It should trace as deleted since it already exists. 2nd execution (transaction) - It should trace nothing as it should be deleted in 1st execution.

For new state object, trace prefix: NEW STATE:

1st execution (transaction) - It should trace as created since it does not exists. 2nd execution (transaction) - It should trace as deleted since it's created in 1st execution. 3rd execution (transaction) - It should trace as created since it's deleted in 2nd execution.

Actual Result

For older state object, trace prefix: OLDER STATE:

1st execution (transaction) - It traces as deleted since it already exists. 2nd execution (transaction) - It still traces as deleted even though it should be deleted in 1st execution and should not be exist now.

For new state object, trace prefix: NEW STATE:

1st execution (transaction) - It traces as created since it does not exists. 2nd execution (transaction) - It traces as deleted since it's created in 1st execution. 3rd execution (transaction) - It traces as created since it's deleted in 2nd execution.

Conclusion

Newly created states are getting deleted properly.
But older states doesn't stay deleted for next executions.

Environment

testnet hooks-chaining branch

RichardAH commented 2 years ago

Please submit fully reproducible code from a genesis ledger including any setup that needs to happen on ledger. Your code assumes a pre-existing "old state" and does nothing on a genesis ledger. Re-open issue with complete code.

RichardAH commented 2 years ago

Hint: your array existing_data should be type uint8_t

XRPL-Labs / xrpld-hooks