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

fix: incremented nonce on out of fund failure #671

Merged
merged 4 commits into from
Feb 1, 2023

Conversation

joshuajbouw
Copy link
Contributor

Description

A bug was found where we increment the nonce when a user is OutOfFund. After looking into it, there were some other areas where the nonce could be incremented incorrectly. As far as I can tell, with these changes, this shouldn't affect internal services, but that needs to be checked ideally by @birchmd.

Performance / NEAR gas cost considerations

No gas increases, though a slight reduction should be possible.

Testing

Tests were updated for this fix.

How should this be reviewed

While the fix should be correct, it should be checked that this is fine for internal services.

@joshuajbouw joshuajbouw added C-bug Category: Something isn't working. P-high Pririoty: high C-incident Category: Issues that are related to or have caused an incident A-engine Area: purely engine EVM related labels Jan 26, 2023
@aleksuss
Copy link
Member

@joshuajbouw take a look, please.

Copy link
Contributor Author

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Good code quality improvements, I appreciate it.

@joshuajbouw joshuajbouw marked this pull request as ready for review January 31, 2023 10:59
engine-tests/src/tests/sanity.rs Outdated Show resolved Hide resolved
engine/src/engine.rs Show resolved Hide resolved
engine/src/engine.rs Show resolved Hide resolved
@aleksuss aleksuss merged commit 60165ad into develop Feb 1, 2023
@aleksuss aleksuss deleted the fix/incremented-nonce-on-out-of-fund branch February 1, 2023 13:56
@joshuajbouw joshuajbouw mentioned this pull request Apr 5, 2023
joshuajbouw added a commit that referenced this pull request Apr 5, 2023
## [2.9.0] 2023-04-05

### Added

- Enabled XCC for mainnet release by [@birchmd]. ([#694])
- Added `set_owner` contract method which sets the owner of the contract
by [@hskang9]. ([#690])
- New variant of submit function `submit_with_args` which accepts
additional arguments along with the transaction such as the max gas
price a user is ready to pay by [@aleksuss]. ([#696])
- Added the ability to create and fund XCC sub-accounts from external
NEAR accounts by [@birchmd]. ([#735])

### Changes

- Replaced `rjson` with `serde_json` by [@aleksuss]. ([#677])
- Changed owner intended contract methods to now require owner or the
contract itself by [@hskang9]. ([#676])

### Fixes

- Fixed nonce incorrectly being incremented on an out of fund failure by
[@joshuajbouw]. ([#671])
- Fixed a check in promise results before executing cross contract calls
(XCC) callbacks by [@birchmd]. ([#693])
- Fixed a reachable panic in `receive_erc20_tokens` by [@0x3bfc].
([#709])
- Fixed a lack of minimum size checks when instantiating a new `EthGas`
object by [@lempire123]. ([#722])
- Fixed a lack of division by 0 checks in `EthGas::Div()` by
[@lempire123]. ([#718])
- Fixed the validation of the return of `exports:storage_remove` by
[@0x3bfc]. ([#712])
- Fixed missing account validations of NEAR account IDs by [@0x3bfc].
([#703])
- Fixed a reachable panic in the `exitToNear` and `exitToEthereum`
precompiles if the input amount is greater than 1^128 when cast from a
`U256` to `u128` by [@0x3bfc]. ([#681])
- Fixed a reachable panic in `modExp` due to arithmetic overflow by
[@0x3bfc]. ([#688])
- Fixed the ability attaching values to Aurora specific precompiles,
this no longer is possible, by [@0x3bfc]. ([#714])
- Fixed a return error if an ecrecover signature length is not exactly
65 by [@0x3bfc]. ([#717])
- Fixed size checks on input array passed to `exitToNear` and
`exitToEthereum` precompiles by [@0x3bfc]. ([#684])
- Fixed missing gas costs in `exitToNear` and `exitToEthereum`
precompiles by [@lempire123]. ([#687])
- Fixed a reachable panic due to out of memory in the `modExp`
precompile by [@0x3bfc]. ([#689])
- Fixed an assurance that the `sender_id` has a balance greater than the
amount in `ft_transfer_call` by [@0x3bfc]. ([#708])
- Fixed returning `0x` when a length cannot be cast as `usize` instead
of returning an error in the `modExp` precompile by [@birchmd]. ([#737])
- Miscellaneous minor fixes by [@0x3bfc]. ([#738])

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Leandro Casuso Montero <leandro.montero@aurora.dev>
Co-authored-by: Michael Birch <michael.birch@aurora.dev>
Co-authored-by: Alexey Lapitsky <lex@realisticgroup.com>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Alexey Lapitsky <alexey.lapitsky@aurora.dev>
Co-authored-by: Hyungsuk Kang <hskang9@gmail.com>
Co-authored-by: Ahmed Ali <ahmedaabdulwahed@gmail.com>
Co-authored-by: lempire123 <61431140+lempire123@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-engine Area: purely engine EVM related C-bug Category: Something isn't working. C-incident Category: Issues that are related to or have caused an incident P-high Pririoty: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants