aurora-is-near / aurora-engine

⚙️ Aurora Engine implements an Ethereum Virtual Machine (EVM) on the NEAR Protocol.
https://doc.aurora.dev/develop/compat/evm
327 stars 80 forks source link

Fix panic in ExitToNear #865

Closed guidovranken closed 10 months ago

guidovranken commented 10 months ago

Description

If input is empty a panic occurs. This PR sets flag to the default value in that case.

Performance / NEAR gas cost considerations

Testing

Fuzzing.

How should this be reviewed

Ensure that flag assuming the default value if input is empty is the intended behavior.

Additional information

aleksuss commented 10 months ago

Yes. But the only problem is that the parse_input returns modified input. I would propose modifying the parse_input function so it will return the flag as well:

#[cfg(feature = "error_refund")]
let (flag, refund_address, mut input) = parse_input(input)?;
#[cfg(not(feature = "error_refund"))]
let (flag, mut input) = parse_input(input)?;
birchmd commented 10 months ago

@aleksuss Yes, that is what I suggested above. My worry about complexity is because of the conditional compilation you would need to change both versions of the parse_input function (duplicating the flag assignment logic). It's not a big deal to duplicate that one line, and I agree it's nicer to encapsulate all the input handling into one function (even if there are two different versions of that function). So I think I agree with you that it's the better solution overall.

aleksuss commented 10 months ago

On the other hand, even if we get the default value for the flag, we will fail later because we don't pass the validation. So, I guess it's absolutely fine and could be merged.