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

Return actual status of a transaction #218

Merged
merged 10 commits into from Aug 6, 2021
Merged

Conversation

joshuajbouw
Copy link
Contributor

@joshuajbouw joshuajbouw commented Aug 5, 2021

In response to the issue #216 that @birchmd had found, it is true that certain errors should still be considered "successful" even though the execution had in fact failed. This fixes that.

Unfortunately, this is a breaking pub API change, but functionally similar.

DO note that this is missing an important test which @birchmd had created as I am unsure if this test is actually correct, but I am currently unsure how such a test could be created.

Closes #216.

@joshuajbouw joshuajbouw added C-bug Category: Something isn't working. P-critical Priority: critical labels Aug 5, 2021
@joshuajbouw joshuajbouw requested a review from artob as a code owner August 5, 2021 11:53
@joshuajbouw joshuajbouw changed the base branch from master to develop August 5, 2021 11:54
@joshuajbouw joshuajbouw marked this pull request as draft August 5, 2021 17:24
@birchmd birchmd marked this pull request as ready for review August 5, 2021 18:32
Copy link
Contributor

@sept-en sept-en left a comment

Choose a reason for hiding this comment

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

Nice, this is really needed! LGTM!

@@ -49,19 +50,32 @@ fn deploy_evm() -> AuroraAccount {
0,
)
.assert_success();
let init_args = InitCallArgs {
prover_account,
eth_custodian_address: "d045f7e19B2488924B97F9c145b5E51D0D895A65".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

How we got this eth_custodian_address?

src/parameters.rs Outdated Show resolved Hide resolved
@joshuajbouw joshuajbouw merged commit 9065842 into develop Aug 6, 2021
birchmd added a commit to aurora-is-near/aurora.js that referenced this pull request Aug 6, 2021
…use old format as fallback for backwards compatibility
artob added a commit that referenced this pull request Aug 14, 2021
* ERC-20: forbid using invalid NEP141 AccountID for mapping (#179)
* Timestamp should be in milliseconds for Ethereum compatibility (#208)
* feat(engine): Blockhash definition (#213)
* Update etc/state-migration-test/Cargo.lock (#211)
* Include cost of access list in intrinsic gas (#219)
* Bump tar from 4.4.13 to 4.4.15 in /etc/eth-contracts (#217)
* Feat(engine): Relayer payment (#215)
* Scheduled lint is supposed to run nightly clippy (#214)
* Return actual status of a transaction (#218)
* Added parser for Integer types (#183)
* Update to latest nightly (#221)
* Fix(engine): do not panic when user has insufficient balance to cover gas (#223)
* Update lock files (#224)
* Method to fix balance of aurora account on testnet (#225)
* Use math api host functions on mainnet (#228)
* NEP-141 compliance correctness (#202)
* Adapt workflows to dockerized runners (#231)
* Move block height to the end of hashed data. (#233)
* Ensure solidity artifacts are always recompiled (#234)
* Prevent test binary from deploying (#237)
* Add removal of eth-contracts to `make clean`
* Remove deploy_code feature gate

Co-authored-by: Dmitry Strokov <dmitry@aurora.dev>
Co-authored-by: Evgeny Ukhanov <evgeny@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>
@joshuajbouw joshuajbouw deleted the fix/state_status branch July 11, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Something isn't working. P-critical Priority: critical
Projects
None yet
Development

Successfully merging this pull request may close these issues.

State changes are not committed when a transaction execution returns an error
3 participants