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

Cap Initialization Fix #9392

Merged
merged 17 commits into from
Jul 6, 2021
Merged

Cap Initialization Fix #9392

merged 17 commits into from
Jul 6, 2021

Conversation

AdityaSripal
Copy link
Member

@AdityaSripal AdityaSripal commented May 25, 2021

Description

Temporarily non-breaking fix for #9324

UNTESTED, should be tested for statesync manually against current Hub

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@amaury1093 amaury1093 added this to the v0.42.6 milestone May 28, 2021
@amaury1093
Copy link
Contributor

Is there any progress on this PR?

We're planning to release v0.42.6 soon (ideally tomorrow), I don't think this will get in, is it fine if it's punted to v0.42.7?

@alexanderbez
Copy link
Contributor

We definitely need to get some sort of fix in. This is an important bug to fix IMHO.

@alexanderbez
Copy link
Contributor

In the interest of time, we can proceed with the global variable approach.

@amaury1093
Copy link
Contributor

In the interest of time, we can proceed with the global variable approach.

@AdityaSripal Should we create a PR on master first and backport this?

@tac0turtle
Copy link
Member

Let's get this in for 0.42.6. This is a major blocker. @AdityaSripal do you have time today? I can test on osmosis later today if it's ready

@amaury1093
Copy link
Contributor

Should we create a PR on master first and backport this?

@tac0turtle
Copy link
Member

Still testing this in the hub. Running into other issues.

I believe This is a middle ground for 0.42. 0.43 should have the actual fix which is breaking

@tac0turtle
Copy link
Member

tac0turtle commented Jul 6, 2021

fix the linting, lets get this merged

@alexanderbez
Copy link
Contributor

fix the linting, lets get this merged

Still failing lint

x/bank/types/balance.go Outdated Show resolved Hide resolved
@alexanderbez alexanderbez merged commit 3776793 into release/v0.42.x Jul 6, 2021
@alexanderbez alexanderbez deleted the aditya/cap-init branch July 6, 2021 13:12
@amaury1093
Copy link
Contributor

@Mergifyio backport release/v0.43.x

mergify bot pushed a commit that referenced this pull request Jul 28, 2021
(cherry picked from commit 3776793)

# Conflicts:
#	x/ibc/applications/transfer/simulation/params.go
@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2021

Command backport release/v0.43.x: success

Backports have been created

@amaury1093
Copy link
Contributor

@Mergifyio backport master

mergify bot pushed a commit that referenced this pull request Jul 28, 2021
(cherry picked from commit 3776793)

# Conflicts:
#	x/ibc/applications/transfer/simulation/params.go
@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2021

Command backport master: success

Backports have been created

amaury1093 added a commit that referenced this pull request Jul 28, 2021
* x/capability: Cap Initialization Fix (#9392)

(cherry picked from commit 3776793)

# Conflicts:
#	x/ibc/applications/transfer/simulation/params.go

* remove ibc file

Co-authored-by: Aditya <adityasripal@gmail.com>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Jul 28, 2021
This PR is a forward-port of #9392 from `release/v0.42.x` to `master`.

It was initially decided in #9392 (comment) that #9392 would be a temporary fix for 0.42, but as per @AdityaSripal's request we're actually foward-porting this to master too.

## Mergify description:

This is an automatic backport of pull request #9392 done by [Mergify](https://mergify.io).
Cherry-pick of 3776793 has failed:
```
On branch mergify/bp/master/pr-9392
Your branch is up to date with 'origin/master'.

You are currently cherry-picking commit 3776793.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   x/bank/simulation/params.go
	modified:   x/capability/keeper/keeper.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   x/ibc/applications/transfer/simulation/params.go

```


To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.io/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.io/) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.io/
</details>
evan-forbes pushed a commit to evan-forbes/cosmos-sdk that referenced this pull request Oct 12, 2021
* x/capability: Cap Initialization Fix (cosmos#9392)

(cherry picked from commit 3776793)

# Conflicts:
#	x/ibc/applications/transfer/simulation/params.go

* remove ibc file

Co-authored-by: Aditya <adityasripal@gmail.com>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>
evan-forbes pushed a commit to evan-forbes/cosmos-sdk that referenced this pull request Nov 1, 2021
* x/capability: Cap Initialization Fix (cosmos#9392)

(cherry picked from commit 3776793)

# Conflicts:
#	x/ibc/applications/transfer/simulation/params.go

* remove ibc file

Co-authored-by: Aditya <adityasripal@gmail.com>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>
Raumo0 pushed a commit to mapofzones/cosmos-sdk that referenced this pull request Feb 13, 2022
* Fixed parse key issue (backport cosmos#9299) (cosmos#9561)

* Fixed parse key issue (cosmos#9299)

* Fixed parse key issue

* Added getconfig in root command

* uncommented changes in parse.go

(cherry picked from commit d7dd1d7)

# Conflicts:
#	simapp/simd/cmd/root.go

* Add changelog

Co-authored-by: Prathyusha Lakkireddy <prathyusha@vitwit.com>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

* feat: return trace value from baseapp (backport cosmos#9578) (cosmos#9580)

* feat: return trace value from baseapp (cosmos#9578)

## Description

Closes: #XXXX

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

* fix: Dumping upgrade info interrupted by cosmovisor (cosmos#9384) (cosmos#9608)

Solution:
- dumping upgrade info before emit `UPGRADED NEEDED` log which will
  cause cosmovisor to kill chain process

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
v    Before smashing the submit button please review the checkboxes.
v    If a checkbox is n/a - please still include it but + a little note why
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Description

The problematic procedure:

1. chain process output UPGRADE NEEDED log
2. cosmovisor see the message and kill the chain binary
3. chain process dump upgrade info and panic itself

the step 2 and 3 runs concurrently, so the dumping process can be interrupted by cosmovisor's terminate signal. The proposed solution is to dump upgrade info before emitting the log.
there are two problematic situation:
1. the upgrade info file is created, but content is not written or flushed before killed, when the chain process restart, it'll panic because of json parsing error.
2. the upgrade info file is not created at all, when the chain process restart, the [store upgrades](https://github.com/crypto-org-chain/chain-main/blob/master/app/app.go#L436) are not activated, will cause app hash mismatch error later on.

* feat: Error on blank chain-id in multisign command (backport cosmos#9593) (cosmos#9605)

* feat: Error on blank chain-id in multisign command (cosmos#9593)

Error on `tx multisign` command if chain-id is blank. This is a common cause of signature verification failures when combining signatures and the error message doesn't provide any clues to this common cause.

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] 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 f65b6c9)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

* less change diff

Co-authored-by: Zaki Manian <zaki@manian.org>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

* x/capability: Cap Initialization Fix (cosmos#9392)

* fix: correct ibc metric labels (cosmos#9645)

* backport cosmos/ibc-go#223

* add changelog

* fix unnecessary changes

* fix: Fix IBC Transfer Event (cosmos#9640)

* fix event type

* CHANGELOG

* Update CHANGELOG.md

Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>

* chore: v0.42.7 release notes & changelog (cosmos#9661)

* chore: v0.42.7 release notes & changelog

* Add ibc

* fix(keyring): update keyring for kwallet fix (backport cosmos#9563) (cosmos#9579)

* fix(keyring): update keyring for kwallet fix (cosmos#9563)

## Description

Closes: cosmos#9562

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Prathyusha Lakkireddy <prathyusha@vitwit.com>
Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Zaki Manian <zaki@manian.org>
Co-authored-by: Aditya <adityasripal@gmail.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
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