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

txIter returns an iterator that runs with constant timestamp even when blockchain.now is manually modified #57

Open
9oelM opened this issue Dec 10, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@9oelM
Copy link

9oelM commented Dec 10, 2024

Describe the bug

Developers have freedom to modify blockchain.now as needed. However, when one wants to use sendMessageIter and update blockchain.now as each message gets executed, all messages will actually have a constant timestamp because of this:

async sendMessageIter(message: Message | Cell, params?: MessageParams): Promise<AsyncIterator<BlockchainTransaction> & AsyncIterable<BlockchainTransaction>> {
params = {
now: this.now,
...params,
}
await this.pushMessage(message)
// Iterable will lock on per tx basis
return await this.txIter(true, params)
}

params will have now: this.now, which will stay constant because this.now is just a plain number, not a reference to an object. As the iterator returned by this function iterates, each transaction executed by this iterator will always have the same timestamp that was given at the beginning, even if blockchain.now is manually updated to something else.

To Reproduce

An example repository has been provided at https://github.com/9oelM/ton-sandbox-bug.git:

git clone https://github.com/9oelM/ton-sandbox-bug.git

npm i

npm test

The test is simple. It sends a message using blockchain.sendMessageIter and executeTill, and checks timestamp at each step. The contracts are configured to ~dump([contractId, now()]) when they receive a message, so we can check the timestamp when the contract actually runs.

const messages = await blockchain.sendMessageIter(
            internal({
                from: increaser.address,
                to: contractA.address,
                value: toNano(`0.5`),
                body: beginCell()
                    .storeUint(Opcodes.increase, 32)
                    .storeUint(0, 64)
                    .storeUint(1, 32)
                .endCell(),
            }),
            {
                now: blockchain.now,
            }
        );

        console.log({
            step: 0,
            from_contract_a: await contractA.getCounterAndTimestamp(),
            from_contract_b: await contractB.getCounterAndTimestamp(),
            blockchain_now: blockchain.now,
        })

        blockchain.now += 1000;

        console.log({
            step: 1,
            from_contract_a: await contractA.getCounterAndTimestamp(),
            from_contract_b: await contractB.getCounterAndTimestamp(),
            blockchain_now: blockchain.now,
        })

        await executeTill(messages, {
            from: increaser.address,
            to: contractA.address,
            success: true,
        });

        blockchain.now += 1000;

        console.log({
            step: 2,
            from_contract_a: await contractA.getCounterAndTimestamp(),
            from_contract_b: await contractB.getCounterAndTimestamp(),
            blockchain_now: blockchain.now,
        })

        const end = await executeTill(messages, {
            from: contractA.address,
            to: contractB.address,
            success: true,
        });

        console.log({
            step: 3,
            from_contract_a: await contractA.getCounterAndTimestamp(),
            from_contract_b: await contractB.getCounterAndTimestamp(),
            blockchain_now: blockchain.now,
        })

The logs show that while blockchain.now increments by 1000, now() called inside the contract returns the initial blockchain.now it started with.

image

Expected behavior
If it were correctly run, the debug logs from the contract must be: #DEBUG#: s0 = [1 1733802917] and #DEBUG#: s0 = [2 1733803917] because the timestamp in the contract needs to be in sync with blockchain.now.

Actual behavior
The timestamp in the contract fails to follow blockchain.now when it gets manually updated.

System information

  • Package version: 0.22.0
  • Node version: 22

Solution
To fix this problem, we just need to introduce a now parameter to next function that will override the original now parameter passed down from params:

protected txIter(needsLocking: boolean, params?: MessageParams): AsyncIterator<BlockchainTransaction> & AsyncIterable<BlockchainTransaction> {
const it = { next: () => this.processTx(needsLocking, params), [Symbol.asyncIterator]() { return it; } }
return it;

Above code must be modified as follows:

    protected txIter(needsLocking: boolean, params?: MessageParams): AsyncIterator<BlockchainTransaction> & AsyncIterable<BlockchainTransaction> {
+        const it = { next: (args?: { now: number }) => this.processTx(needsLocking, { ...params, ...(args ? args : {})  }), [Symbol.asyncIterator]() { return it; } }
-        const it = { next: () => this.processTx(needsLocking, params), [Symbol.asyncIterator]() { return it; } }
        return it;
    }

Also, because executeTill and executeFrom depend on next(), they need to have an additional parameter specifying now too:

+ export async function executeTill<T extends Transaction>(txs: AsyncIterator<T>, match: FlatTransactionComparable, args?: { now: number }) {
- export async function executeTill<T extends Transaction>(txs: AsyncIterator<T>, match: FlatTransactionComparable) {
    let executed: T[] = [];
+      let iterResult = await txs.next(args);
-    let iterResult = await txs.next();
    let found = false;
    while (!iterResult.done) {
        executed.push(iterResult.value);
        found = compareTransaction(flattenTransaction(iterResult.value), match);
        if (found) {
            break;
        }
+        iterResult = await txs.next(args);        
-        iterResult = await txs.next();
    }
    if (!found) {
        throw new Error(`Expected ${inspect(executed.map(x => flattenTransaction(x)))} to contain a transaction that matches pattern ${inspect(match)}`);
    }

    return executed;
}

+ export async function executeFrom<T extends Transaction>(txs: AsyncIterator<T>, args?: { now: number }) {
- export async function executeFrom<T extends Transaction>(txs: AsyncIterator<T>) {
    let executed: T[] = [];
+   let iterResult = await txs.next(args);
-   let iterResult = await txs.next();

    while (!iterResult.done) {
        executed.push(iterResult.value);
+       iterResult = await txs.next(args);
-       iterResult = await txs.next();
    }

    return executed;
}

Or another way to handle this would be to read from blockchain.now directly and override it. Thie approach would not require exeuteTill and executeFrom to be fixed, because next() will always follow the latest blockchain.now:

    protected txIter(needsLocking: boolean, params?: MessageParams): AsyncIterator<BlockchainTransaction> & AsyncIterable<BlockchainTransaction> {
+        const it = { next: () => this.processTx(needsLocking, { ...params, now: this.now }), [Symbol.asyncIterator]() { return it; } }
-        const it = { next: () => this.processTx(needsLocking, params), [Symbol.asyncIterator]() { return it; } }
        return it;
    }

Ultimately, this depends on the design decision of the SDK.

Anyway, once you fix things, you will be able to see the test working correctly as intended:

image

@9oelM 9oelM added the bug Something isn't working label Dec 10, 2024
@9oelM
Copy link
Author

9oelM commented Dec 10, 2024

FYI I couldn't wait for an official fix, so I've published a fix at https://github.com/blubbofi/sandbox. Can install with npm i @blubbofi/sandbox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant