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.

Sign up for GitHub

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

Jump to bottom

fix: Revert "fix: halt-height behavior is not deterministic (#305)" #337

Merged
merged 3 commits into from
Nov 9, 2023
Merged

fix: Revert "fix: halt-height behavior is not deterministic (#305)" #337

merged 3 commits into from
Nov 9, 2023

Conversation

JimLarson
Copy link

@JimLarson JimLarson commented Nov 9, 2023 •
edited
Loading

Description

Refs Agoric/agoric-sdk#8223
Refs Agoric/agoric-sdk#8326

Revert #305 which causes test failure in Agoric/agoric-sdk.

Author Checklist

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • run make lint and make test
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@JimLarson
Revert "fix: halt-height behavior is not deterministic (#305)"
587d599 
This reverts commit 7b8423a.

This commit caused test failures in agoric-sdk.
Reverting to land other changes in agoric-sdk.
We'll restore the change and try to debug the failure
against a smaller diff in the future.
@JimLarson JimLarson self-assigned this Nov 9, 2023
@JimLarson JimLarson changed the title Revert "fix: halt-height behavior is not deterministic (#305)" fix: Revert "fix: halt-height behavior is not deterministic (#305)" Nov 9, 2023
@JimLarson JimLarson added agoric-cosmos Agoric tags for cosmos bug Something isn't working labels Nov 9, 2023
@JimLarson
Copy link
Author

Anticipating an immediate tag.

@JimLarson JimLarson marked this pull request as ready for review November 9, 2023 01:41
@michaelfig
Copy link
Collaborator

Weird golangci-lint failure. Ugh.

michaelfig
michaelfig approved these changes Nov 9, 2023
View reviewed changes
Copy link
Collaborator

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

mhofman
mhofman reviewed Nov 9, 2023
View reviewed changes
CHANGELOG-Agoric.md Outdated
@@ -45,4 +51,3 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Bug Fixes

* (snapshots) [#304](https://github.com/agoric-labs/cosmos-sdk/pull/304) raise the per snapshot item limit. Fixes [Agoric/agoric-sdk#8325](https://github.com/Agoric/agoric-sdk/issues/8325)
* (baseapp) [#305](https://github.com/agoric-labs/cosmos-sdk/pull/305) Make sure we don't execute blocks beyond the halt height. Port of [cosmos/cosmos-sdk#16639](https://github.com/cosmos/cosmos-sdk/pull/16639)
Copy link

Choose a reason for hiding this comment

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

We probably shouldn't delete this entry

Copy link
Author

Choose a reason for hiding this comment

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

Restored.

@michaelfig michaelfig mentioned this pull request Nov 9, 2023
Merged
20 tasks
@JimLarson JimLarson merged commit 30e4a2e into Agoric Nov 9, 2023
30 of 31 checks passed
@JimLarson JimLarson deleted the 8223-revert-halt branch November 9, 2023 07:26
JeancarloBarrios pushed a commit that referenced this pull request Sep 28, 2024
@JimLarson @JeancarloBarrios
fix: Revert "fix: halt-height behavior is not deterministic (#305)" (#…
dfd8f6c 
…337)

* Revert "fix: halt-height behavior is not deterministic (#305)"

This reverts commit 7b8423a.

This commit caused test failures in agoric-sdk.
Reverting to land other changes in agoric-sdk.
We'll restore the change and try to debug the failure
against a smaller diff in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhofman mhofman mhofman left review comments

@michaelfig michaelfig michaelfig approved these changes

Assignees

@JimLarson JimLarson

Labels
agoric-cosmos Agoric tags for cosmos bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JimLarson @michaelfig @mhofman