Aepp-sdk-python, does waiting for confirmation guarantee that transaction is on current chain?

Hello,

I was reading docs and I feel like there is something amiss here:

https://aepp-sdk-python.readthedocs.io/en/latest/_modules/aeternity/node.html#NodeClient.wait_for_confirmation

    def wait_for_confirmation(self, tx_hash, max_retries=None, polling_interval=None):
        """
        Wait for a transaction to be confirmed by at least "key_block_confirmation_num" blocks (default 3)
        The amount of blocks can be configured in the Config object using key_block_confirmation_num parameter

        ...

        """

We are waiting for a transaction to be confirmed by at least few blocks:

        # first wait for the transaction to be found
        tx_height = self.wait_for_transaction(tx_hash)
        # now calculate the min block height
        min_block_height = tx_height + self.config.key_block_confirmation_num

So far looks OK. And then we are waiting for the blockchain to grow by few blocks:

        while True:
            current_height = self.get_current_key_block_height()
            # if the tx.block_height >= min_block_height we are ok
            if current_height >= min_block_height:
                break
            if n >= retries:
                raise TransactionWaitTimeoutExpired(tx_hash=tx_hash, reason=f"The transaction was not included in {total_sleep} seconds, wait aborted")
            # calculate sleep time
            time.sleep(interval)

I believe at this point we have no guarantees that the transaction is actually included in the current chain - we could already be at another fork. Whole point of waiting for confirmations is to protect against double spending.

Is there another protection, somewhere deeper?

1 Like

Hi @hiciu,

I am not familiar with the Python SDK but looking at the code, it looks like this is indeed a bug. The code should check if the transaction had been rejected by the chain and simply observing the chain height does not do that.

cc @noandrea @dincho.chain @yani.chain

Edit: added Yani as well for better communication

hey @hiciu, thanks for the report!
I have tracked the issue here: https://github.com/aeternity/aepp-sdk-python/issues/349
If you want to submit a PR for this one it would be super appreciated!

3 Likes