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

1726 marker creation missing metadata #1728

Merged
merged 13 commits into from
Nov 3, 2023

Conversation

nullpointer0x00
Copy link
Contributor

Description

Marker client id was '07-tendermint-71' and causing the issue. Decided just cast it and if not tendermint return "unknown".

closes: #1726


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

@nullpointer0x00 nullpointer0x00 requested a review from a team as a code owner November 1, 2023 20:45
@nullpointer0x00 nullpointer0x00 self-assigned this Nov 1, 2023
@nullpointer0x00 nullpointer0x00 added ibc inter-blockchain communication ibc-hooks labels Nov 1, 2023
@nullpointer0x00 nullpointer0x00 added this to the backlog milestone Nov 1, 2023
iramiller
iramiller previously approved these changes Nov 1, 2023
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #1728 (de189b5) into main (8d52e13) will decrease coverage by 0.06%.
The diff coverage is 36.17%.

❗ Current head de189b5 differs from pull request most recent head bc77205. Consider uploading reports for the commit bc77205 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1728      +/-   ##
==========================================
- Coverage   68.77%   68.72%   -0.06%     
==========================================
  Files         288      288              
  Lines       36483    36522      +39     
==========================================
+ Hits        25092    25099       +7     
- Misses      10039    10069      +30     
- Partials     1352     1354       +2     
Files Coverage Δ
x/ibchooks/marker_hooks.go 69.94% <100.00%> (-0.75%) ⬇️
app/upgrades.go 62.10% <25.00%> (-7.34%) ⬇️

Taztingo
Taztingo previously approved these changes Nov 1, 2023
@nullpointer0x00 nullpointer0x00 dismissed stale reviews from Taztingo and iramiller via 67f9e74 November 1, 2023 21:40
app/upgrades.go Outdated Show resolved Hide resolved
…reate test for rc2 and update existing tests to have logs.
app/upgrades.go Outdated Show resolved Hide resolved
app/upgrades.go Outdated Show resolved Hide resolved
@Taztingo Taztingo self-assigned this Nov 2, 2023
@Taztingo Taztingo marked this pull request as draft November 2, 2023 20:58
@Taztingo Taztingo marked this pull request as ready for review November 2, 2023 21:29
@Taztingo Taztingo enabled auto-merge (squash) November 2, 2023 21:30
@Taztingo Taztingo merged commit 647a723 into main Nov 3, 2023
34 checks passed
@Taztingo Taztingo deleted the nullpointer0x00/1726-marker-creation-missing-metadata branch November 3, 2023 12:55
SpicyLemon pushed a commit that referenced this pull request Nov 3, 2023
* cast client state to 07-tendermint obj, fail to unknown

* return chainid instead of assign

* update comment

* change GetChainID method signature, add upgrade handler for creating denom metadata

* fix denom setting in markerMetadata

* Add updateIbcMarkerDenomMetadata to saffron and add logs to method. Create test for rc2 and update existing tests to have logs.

* Add fix to get correct denom trace.

* Change panics to logs. Remove some indentation that is not needed.

* Remove chain from description.

* Remove chain from description in hooks. Use BankKeeper.SetDenomMetadata.

* Update tests

---------

Co-authored-by: Matthew Witkowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ibc inter-blockchain communication ibc-hooks
Projects
Development

Successfully merging this pull request may close these issues.

IBC marker creation missing metadata
4 participants