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

Fixed the rollback functionality to work with empty gas price database #2105

Merged
merged 8 commits into from
Aug 20, 2024

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Aug 19, 2024

Closes #2104

Checklist

  • New behavior is reflected in tests

Before requesting review

  • I have reviewed the code myself

@xgreenx xgreenx requested a review from a team August 19, 2024 16:56
@xgreenx xgreenx self-assigned this Aug 19, 2024
Comment on lines 257 to 259
let gas_price_rollbacked = gas_price_chain_height.is_none()
|| gas_price_chain_height.expect("We checked height before")
== target_block_height;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let gas_price_rollbacked = gas_price_chain_height.is_none()
|| gas_price_chain_height.expect("We checked height before")
== target_block_height;
let gas_price_rolled_back = gas_price_chain_height.is_none()
|| gas_price_chain_height.expect("We checked height before")
== target_block_height;

Copy link
Member

Choose a reason for hiding this comment

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

roll_backed vs rolled_back, I think latter is preferred


if on_chain_height == target_block_height
&& off_chain_height == target_block_height
&& gas_price_chain_height == target_block_height
&& gas_price_rollbacked
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&& gas_price_rollbacked
&& gas_price_rolled_back


// When
let args = [
"_IGNORED_",
Copy link
Member

Choose a reason for hiding this comment

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

I've seen this being used in other tests too, what does this mean?

Copy link
Member

Choose a reason for hiding this comment

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

That's the first (shell) command argument, which is usually the path to the binary. Since the executable name isn't used for anything and pointing to the real fuel-core binary would make no sense, just having a dummy string here is fine.

Dentosal
Dentosal previously approved these changes Aug 20, 2024
Dentosal
Dentosal previously approved these changes Aug 20, 2024
…atabase-doesnt-exist' into bugfix/rollback-when-gas-price-database-doesnt-exist
@xgreenx xgreenx requested review from Dentosal and a team August 20, 2024 10:37
@xgreenx xgreenx merged commit 9a9e42e into master Aug 20, 2024
31 checks passed
@xgreenx xgreenx deleted the bugfix/rollback-when-gas-price-database-doesnt-exist branch August 20, 2024 11:40
@xgreenx xgreenx mentioned this pull request Aug 20, 2024
xgreenx added a commit that referenced this pull request Aug 20, 2024
## Version v0.34.0

### Added
- [2051](#2051): Add support
for AWS KMS signing for the PoA consensus module. The new key can be
specified with `--consensus-aws-kms AWS_KEY_ARN`.
- [2092](#2092): Allow
iterating by keys in rocksdb, and other storages.
- [2096](#2096): GraphQL
endpoint to fetch blob byte code by its blob ID.

### Changed
- [2106](#2106): Remove
deadline clock in POA and replace with tokio time functions.

#### Breaking
- [2051](#2051): Misdocumented
`CONSENSUS_KEY` environ variable has been removed, use
`CONSENSUS_KEY_SECRET` instead. Also raises MSRV to `1.79.0`.

### Fixed

- [2106](#2106): Handle the
case when nodes with overriding start on the fresh network.
- [2105](#2105): Fixed the
rollback functionality to work with empty gas price database.

## What's Changed
* doc: refine the transaction example in the README by @mmyyrroonn in
#2072
* AWS KMS block signing support and Rust 1.79 by @Dentosal in
#2051
* feat(iterators): allow key-only iteration by @rymnc in
#2092
* feat: graphql endpoint to fetch the blob byte code by its blob ID by
@netrome in #2096
* Small improvements for tests to make them more stable by @xgreenx in
#2103
* Fixed the rollback functionality to work with empty gas price database
by @xgreenx in #2105
* Bump wasmtime version by @Dentosal in
#2089
* Handle the case when nodes with overriding start on the fresh network
by @xgreenx in #2106
* Remove deadline clock in POA and replace with tokio time functions. by
@AurelienFT in #2109

## New Contributors
* @mmyyrroonn made their first contribution in
#2072

**Full Changelog**:
v0.33.0...v0.34.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignore the gas price database during rollback if it is empty
3 participants