fix: populate ctx.ConsensusParams for begin blockers #10725

yihuang · 2021-12-10T09:35:39Z

Description

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...

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)

tac0turtle

LGTM.

Seems no apps are using consensus params in their modules 😜

Can you add a changelog entry please

amaury1093

thanks a lot! This makes sense. Can we add:

one small test
changelog entry?

(removing automerge for now)

amaury1093 · 2021-12-10T12:03:47Z

baseapp/abci.go

@@ -176,6 +176,9 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 		WithBlockGasMeter(gasMeter).
 		WithHeaderHash(req.Hash)

+	app.deliverState.ctx = app.deliverState.ctx.
+		WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))


Do we still need

cosmos-sdk/baseapp/baseapp.go

Line 491 in b3e922d

ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))

?

Do we need it for checkTx?

added a condition to it.

Just add WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx)) with the above app.deliverState.ctx. No need for a separate call.

I was worried that the GetConsensusParams might get a uninitialized ctx when I write in that way. The gas meter and header hash not initialized.

In that way, the ctx passed to GetConsensusParams is not initialized with gas meter and block header hash, is that a problem?

No, it's not a problem.

No, it's not a problem.

👍 changed.

yihuang · 2021-12-10T13:52:02Z

thanks a lot! This makes sense. Can we add:

one small test

changelog entry?

(removing automerge for now)

unit test and changelog added.

alexanderbez · 2021-12-10T14:41:52Z

baseapp/abci.go

@@ -176,6 +176,9 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 		WithBlockGasMeter(gasMeter).
 		WithHeaderHash(req.Hash)

+	app.deliverState.ctx = app.deliverState.ctx.
+		WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))


Just add WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx)) with the above app.deliverState.ctx. No need for a separate call.

baseapp/baseapp.go

Closes: cosmos#10724 changelog and unit test revert change in getContextForTx

amaury1093

lgtm

alexanderbez · 2021-12-11T22:13:09Z

baseapp/abci.go

@@ -176,6 +176,9 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 		WithBlockGasMeter(gasMeter).
 		WithHeaderHash(req.Hash)

+	app.deliverState.ctx = app.deliverState.ctx.
+		WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))


No, it's not a problem.

Closes: #10724 ## Description --- ### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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) (cherry picked from commit 675be9d) # Conflicts: # CHANGELOG.md

#10780) * fix: populate ctx.ConsensusParams for begin blockers (#10725) Closes: #10724 ## Description --- ### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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) (cherry picked from commit 675be9d) # Conflicts: # CHANGELOG.md * fix cl * remove test Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

#10781) * fix: populate ctx.ConsensusParams for begin blockers (#10725) Closes: #10724 ## Description --- ### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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) (cherry picked from commit 675be9d) # Conflicts: # CHANGELOG.md * fix cl * remove test Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

#10782) * fix: populate ctx.ConsensusParams for begin blockers (#10725) Closes: #10724 ## Description --- ### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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) (cherry picked from commit 675be9d) # Conflicts: # CHANGELOG.md * fix cl Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

tac0turtle · 2021-12-28T10:49:29Z

@Mergifyio backport release/v0.45.x

mergify · 2021-12-28T10:50:02Z

backport release/v0.45.x

✅ Backports have been created

#10780 fix: populate ctx.ConsensusParams for begin blockers (backport #10725) has been created for branch release/v0.45.x

…#10725) (#10781)" This reverts commit f02c26c.

…#10725) (cosmos#10780) * fix: populate ctx.ConsensusParams for begin blockers (cosmos#10725) Closes: cosmos#10724 ## Description --- ### 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] 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](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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) (cherry picked from commit 675be9d) # Conflicts: # CHANGELOG.md * fix cl * remove test Co-authored-by: yihuang <huang@crypto.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

…cosmos#10725) (cosmos#10781)" This reverts commit f02c26c.

yihuang requested review from aaronc and alexanderbez as code owners December 10, 2021 09:35

tac0turtle approved these changes Dec 10, 2021

View reviewed changes

tac0turtle changed the title ~~populate ctx.ConsensusParams for begin blockers~~ fix: populate ctx.ConsensusParams for begin blockers Dec 10, 2021

tac0turtle added A:automerge Automatically merge PR once all prerequisites pass. backport/0.42.x (Stargate) labels Dec 10, 2021

tac0turtle requested review from amaury1093 and robert-zaremba December 10, 2021 11:07

amaury1093 approved these changes Dec 10, 2021

View reviewed changes

amaury1093 removed the A:automerge Automatically merge PR once all prerequisites pass. label Dec 10, 2021

amaury1093 reviewed Dec 10, 2021

View reviewed changes

alexanderbez suggested changes Dec 10, 2021

View reviewed changes

yihuang mentioned this pull request Dec 13, 2021

Problem: no integration test for EIP-1559 feature crypto-org-chain/cronos#249

Merged

13 tasks

populate ctx.ConsensusParams for begin blockers

7cc6894

Closes: cosmos#10724 changelog and unit test revert change in getContextForTx

yihuang mentioned this pull request Dec 13, 2021

fix: wrong gas limit when adjust base fee evmos/ethermint#832

Merged

11 tasks

yihuang force-pushed the consensus-params branch from ce50fcf to 7cc6894 Compare December 13, 2021 08:45

yihuang requested a review from alexanderbez December 13, 2021 09:19

amaury1093 approved these changes Dec 13, 2021

View reviewed changes

alexanderbez approved these changes Dec 13, 2021

View reviewed changes

alexanderbez and others added 3 commits December 13, 2021 10:56

Merge branch 'master' into consensus-params

83322ab

chain the WithConsensusParams call

b781705

Merge branch 'master' into consensus-params

f951e03

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Dec 14, 2021

amaury1093 and others added 3 commits December 14, 2021 12:06

Merge branch 'master' into consensus-params

d1815df

Merge branch 'master' into consensus-params

4f20fcd

Merge branch 'master' into consensus-params

fdacf83

mergify bot merged commit 675be9d into cosmos:master Dec 15, 2021

mergify bot mentioned this pull request Dec 15, 2021

fix: populate ctx.ConsensusParams for begin blockers (backport #10725) #10780

Merged

mergify bot mentioned this pull request Dec 15, 2021

fix: populate ctx.ConsensusParams for begin blockers (backport #10725) #10781

Merged

mergify bot mentioned this pull request Dec 15, 2021

fix: populate ctx.ConsensusParams for begin blockers (backport #10725) #10782

Merged

yihuang deleted the consensus-params branch December 16, 2021 04:08

fedekunze mentioned this pull request Dec 28, 2021

Limited 0.45 Cosmos SDK release #10656

Closed

12 tasks

faddat mentioned this pull request Feb 28, 2022

v0.45.x akash-network/cosmos-sdk#176

Closed

8 tasks

yihuang added a commit that referenced this pull request Mar 14, 2022

Revert "fix: populate ctx.ConsensusParams for begin blockers (backport …

e0fe179

…#10725) (#10781)" This reverts commit f02c26c.

tomtau added a commit to crypto-org-chain/cosmos-sdk that referenced this pull request Sep 2, 2022

Revert "fix: populate ctx.ConsensusParams for begin blockers (backport …

862d2d9

…cosmos#10725) (cosmos#10781)" This reverts commit f02c26c.

coderabbitai bot mentioned this pull request Apr 19, 2024

Problem: panic on invalid elasticity_multiplier crypto-org-chain/cronos#1397

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: populate ctx.ConsensusParams for begin blockers #10725

fix: populate ctx.ConsensusParams for begin blockers #10725

yihuang commented Dec 10, 2021

tac0turtle left a comment

amaury1093 left a comment •

edited

Loading

amaury1093 Dec 10, 2021

yihuang Dec 10, 2021

yihuang Dec 10, 2021

alexanderbez Dec 10, 2021

yihuang Dec 10, 2021

yihuang Dec 10, 2021

alexanderbez Dec 11, 2021

yihuang Dec 14, 2021

yihuang commented Dec 10, 2021

alexanderbez Dec 10, 2021

amaury1093 left a comment

alexanderbez Dec 11, 2021

tac0turtle commented Dec 28, 2021

mergify bot commented Dec 28, 2021

fix: populate ctx.ConsensusParams for begin blockers #10725

fix: populate ctx.ConsensusParams for begin blockers #10725

Conversation

yihuang commented Dec 10, 2021

Description

Author Checklist

Reviewers Checklist

tac0turtle left a comment

Choose a reason for hiding this comment

amaury1093 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang commented Dec 10, 2021

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tac0turtle commented Dec 28, 2021

mergify bot commented Dec 28, 2021

✅ Backports have been created

amaury1093 left a comment •

edited

Loading