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

fix missing filter codeId in long type #262

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

jiqiang90
Copy link
Contributor

@jiqiang90 jiqiang90 commented Jun 12, 2024

Description

Background: cosmosjs deprecated long for message.callId in 0.32.0 https://github.com/cosmos/cosmjs/blob/main/CHANGELOG.md#changed-2. We made change to remove it in previous pr ac68f61#diff-7ed3b4e1e7a4809a4c839b761afb2eb1a0c45c6b5dd0f01c9cdb520cb80a015bL78.

From v3.11.0, We now facing any issue message filter not able to filter callId and handlers are not triggered.
In this fix, we temp bring back long for message.callId, due to we have multiple version of @cosmjs/stargate installed in our sdk, and older version were installed from dependency @kyvejs/xxx. Use resolution in package.json also could not fixed version for @cosmjs/stargate.

Fixes # (issue)

We shall remove long again once @kyvejs/xxx update dependencies @cosmjs/stargate to 0.32.0 or above.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • I have tested locally
  • I have performed a self review of my changes
  • Updated any relevant documentation
  • Linked to any relevant issues
  • I have added tests relevant to my changes
  • Any dependent changes have been merged and published in downstream modules
  • My code is up to date with the base branch
  • I have updated relevant changelogs. We suggest using chan

@jiqiang90 jiqiang90 requested a review from stwiname June 12, 2024 02:22
Copy link

Coverage report

Caution

Test run failed

St.
Category Percentage Covered / Total
🔴 Statements 46.91% 2460/5244
🟡 Branches 77.04% 245/318
🔴 Functions 45.87% 100/218
🔴 Lines 46.91% 2460/5244

Test suite run failed

Failed tests: 1/64. Failed suites: 1/7.
  ● KyveApi › able to fetch/write/read blocks using Kyve api

    ENOENT: no such file or directory, open '/tmp/8kCSua/bundle_2_0.json'



Report generated by 🧪jest coverage report action from 1385b0e

Copy link
Contributor

@stwiname stwiname left a comment

Choose a reason for hiding this comment

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

We need to understand why we need to revert this. Cosmjs has dropped support for long since 0.32.0 maybe there is something else in the process that needs to be updated rather than reverting this change

Can you please also fill out the PR template future reference.

@jiqiang90 jiqiang90 merged commit 6a33c03 into main Jun 12, 2024
2 of 3 checks passed
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.

2 participants