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

chore: v0.45.0 Release Notes #10760

Merged
merged 8 commits into from
Jan 12, 2022
Merged

chore: v0.45.0 Release Notes #10760

merged 8 commits into from
Jan 12, 2022

Conversation

amaury1093
Copy link
Contributor

@amaury1093 amaury1093 commented Dec 13, 2021

Description

Closes: #10656

Pending on:


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

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
  • 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)

@amaury1093
Copy link
Contributor Author

The 3 pending pieces are all backportable. I prefer to merge and release ASAP, and figure out those pieces later. Thoughts @robert-zaremba ?

@tomtau
Copy link
Contributor

tomtau commented Dec 14, 2021

@AmauryM also #10770

@tomtau
Copy link
Contributor

tomtau commented Dec 14, 2021

As this issue may also affect e.g. CosmWasm: #10769 FYI @ethanfrey

@amaury1093
Copy link
Contributor Author

I was ready to release this today. If we include #10770, then it'll need another round of QA. At regen we'll be off from this Friday, which means 0.45 will be out in Jan.

Happy to delay limited 0.45 if that's okay with everyone.

@ethanfrey
Copy link
Contributor

I see no tests in #10770 and hard to see the implications.

Currently (without the patch), if I make a contract that, says, does an infinite loop and pay 100 million gas, while the block limit is 50 million, this will:

  1. Run for 100 million gas, then hit out of gas, aborting the DeliverTx with a panic
  2. In (txh recoveryTxHandler) DeliverTx it will then try to charge 100 million gas to a 50 million gas meter, causing another panic
  3. What happens to the node?

This panic will abort all state writes in this transaction, and it will end up as an error over the abci interface (probably internal / code id = 1). That is fine.

Also, if you had 3 more transactions in the block after this one, what happens? They all should abort before running (as 0 gas left).

Personally, I am happy to have the 0.45 out this week unless the bug fix is truly critical. The whole block gas thing is a bit confusing to me in general (that we allow way too many tx in the block, then error when running them)

@tomtau
Copy link
Contributor

tomtau commented Dec 15, 2021

@ethanfrey

This panic will abort all state writes in this transaction, and it will end up as an error over the abci interface (probably internal / code id = 1).

The trouble is that only the latter part happens, i.e. the state changes from that tx are persisted, but an error is returned (and no events are emitted). See #10769 (comment)

@ethanfrey
Copy link
Contributor

@ethanfrey

This panic will abort all state writes in this transaction, and it will end up as an error over the abci interface (probably internal / code id = 1).

The trouble is that only the latter part happens, i.e. the state changes from that tx are persisted, but an error is returned (and no events are emitted). See #10769 (comment)

Thanks for the link. I understand it. That is very undesirable behaviour. Not sure if it is exploitable, but it is quite an annoying bug and I wouldn't expect this to happen.

@tomtau
Copy link
Contributor

tomtau commented Jan 3, 2022

@AmauryM also #10770

the version backported to 0.45: #10814

@amaury1093 amaury1093 marked this pull request as ready for review January 12, 2022 16:45
@amaury1093 amaury1093 requested review from robert-zaremba and tac0turtle and removed request for aaronc and alexanderbez January 12, 2022 16:46
Copy link
Collaborator

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

pre-approving

CHANGELOG.md Show resolved Hide resolved
RELEASE_NOTES.md Outdated Show resolved Hide resolved
RELEASE_NOTES.md Outdated Show resolved Hide resolved
RELEASE_NOTES.md Outdated Show resolved Hide resolved
RELEASE_NOTES.md Outdated Show resolved Hide resolved
amaury1093 and others added 5 commits January 12, 2022 18:11
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
@amaury1093 amaury1093 merged commit c1c1ad7 into release/v0.45.x Jan 12, 2022
@amaury1093 amaury1093 deleted the am/v0450-cl-rl branch January 12, 2022 17:32
@amaury1093 amaury1093 added this to the v0.45.0 milestone Jan 12, 2022
@faddat faddat mentioned this pull request Feb 28, 2022
8 tasks
JimLarson pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Jul 7, 2022
* chore: v0.45.0 Release Notes

* Update

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* address review

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Eengineer1 pushed a commit to cheqd/cosmos-sdk that referenced this pull request Aug 26, 2022
* chore: v0.45.0 Release Notes

* Update

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update RELEASE_NOTES.md

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* address review

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
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.

5 participants