ERC-20: change admin address#184
Conversation
joshuajbouw
force-pushed
the
develop
branch
2 times, most recently
from
216e375 to
9d09598
Compare
sept-en
left a comment
sept-en
left a comment
There was a problem hiding this comment.
Thanks @birchmd !
@joshuajbouw please review
| match submit_result.status { | ||
| TransactionStatus::Succeed(_) => Ok(()), | ||
| TransactionStatus::Revert(bytes) => { | ||
| let error_message = crate::prelude::format!( | ||
| "Reverted with message: {}", | ||
| String::from_utf8_lossy(&bytes) | ||
| ); | ||
| Err(EngineError { | ||
| kind: EngineErrorKind::EvmError(ExitError::Other(Cow::from( | ||
| error_message, | ||
| ))), | ||
| gas_used: submit_result.gas_used, | ||
| }) | ||
| } | ||
| TransactionStatus::OutOfFund => Err(EngineError { | ||
| kind: EngineErrorKind::EvmError(ExitError::OutOfFund), | ||
| gas_used: submit_result.gas_used, | ||
| }), | ||
| TransactionStatus::OutOfOffset => Err(EngineError { | ||
| kind: EngineErrorKind::EvmError(ExitError::OutOfOffset), | ||
| gas_used: submit_result.gas_used, | ||
| }), | ||
| TransactionStatus::OutOfGas => Err(EngineError { | ||
| kind: EngineErrorKind::EvmError(ExitError::OutOfGas), | ||
| gas_used: submit_result.gas_used, | ||
| }), | ||
| TransactionStatus::CallTooDeep => Err(EngineError { | ||
| kind: EngineErrorKind::EvmError(ExitError::CallTooDeep), | ||
| gas_used: submit_result.gas_used, | ||
| }), | ||
| } |
There was a problem hiding this comment.
The only thing that I can comment on is that this feels weird to me.
There was a problem hiding this comment.
The reason for this mapping is to turn any failed status into a proper error, which will cause the NEAR transaction to fail. The reason we want to fail the NEAR transaction in this case, but not in the case of submit calls is because this is internal to Aurora. There is no nonce that needs to be incremented or EVM gas that needs to be charged, so we do not need to worry about state being reverted because the transaction fails.
It should also be true that in production none of these failure statuses ever come up (again because its an internal call). But I thought it was still better to catch and properly report them rather than silently swallowing failures, just to be safe.
There was a problem hiding this comment.
No no I get the logic behind it. It just feels like we need a From<T> type of implementation given its structure but that isn't exactly possible.
* ERC-20: change admin address (#184) * Fix promise, broken in upstream (#227) * Feat(tests): View call profiling (#250) * Fix evm_bully feature enabling in Makefile (#252) * Feat(benches): Profile NFT paginated view call (#253) * Update CODEOWNERS (#255) * Bump @openzeppelin/contracts from 4.1.0 to 4.3.1 in /etc/eth-contracts (#254) * Bump tar from 4.4.15 to 4.4.19 in /etc/eth-contracts (#257) * Upgrade nearcore dependencies to latest commit (#259) * 1inch deploy benchmark (#261) Co-authored-by: Dmitry Strokov <dmitry@aurora.dev> Co-authored-by: Joshua J. Bouw <joshua@aurora.dev> Co-authored-by: Kirill <kirill@aurora.dev> Co-authored-by: Michael Birch <michael@aurora.dev> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
4 participants
Fixes #175