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

TOB-FUEL-22: TRO opcode does not revert when amount is zero as specified #563

Closed
xgreenx opened this issue Aug 28, 2023 · 1 comment
Closed
Labels
audit-report Issue from the audit report

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Aug 28, 2023

Description

The transfer output (TRO) opcode transfers coins to addresses by creating a UTXO. The Fuel VM specification states that TRO should panic if the amount ($rC) is zero but the implementation does not perform this validation. Note, that the transfer coins to contract (TR) opcode does perform this validation and will revert for transfers with an amount of zero.

Figure 22.1: The TRO implementation (fuel-vm/src/interpreter/contract.rs#290–367)

   pub(crate) fn transfer_output(
        self,
        a: Word,
        b: Word,
        c: Word,
        d: Word,
    ) -> Result<(), RuntimeError>
    where
        Tx: ExecutableTransaction,
        S: ContractsAssetsStorage,
        <S as StorageInspect<ContractsAssets>>::Error: Into<std::io::Error>,
{
[...]
        let amount = c;
[...]
        // credit variable output
        let variable = Output::variable(to, amount, asset_id);
        set_variable_output(self.tx, self.memory, self.tx_offset, out_idx,
variable)?;
[...]

Recommendations

Short term, determine whether zero amount transfers will cause a panic and update the specifications and implementations to be consistent.
Long term, create unit test cases that trigger all specified panics.

@xgreenx xgreenx added the audit-report Issue from the audit report label Aug 28, 2023
@xgreenx
Copy link
Collaborator Author

xgreenx commented Aug 28, 2023

Is a duplicate of the #522 and fixed by #532

@xgreenx xgreenx closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-report Issue from the audit report
Projects
None yet
Development

No branches or pull requests

1 participant