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

rosetta: upgrade to newest version #9314

Merged
merged 13 commits into from
May 14, 2021
Merged

rosetta: upgrade to newest version #9314

merged 13 commits into from
May 14, 2021

Conversation

noandrea
Copy link
Contributor

@noandrea noandrea commented May 12, 2021

Description

Embed the Rosetta SDK integration within the SDK

closes: #9300


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

@github-actions github-actions bot added C:Rosetta Issues and PR related to Rosetta T: ADR An issue or PR relating to an architectural decision record T:Docs Changes and features related to documentation. labels May 12, 2021
@lgtm-com
Copy link

lgtm-com bot commented May 12, 2021

This pull request introduces 1 alert when merging d494756 into b4125d1 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to field

@codecov
Copy link

codecov bot commented May 12, 2021

Codecov Report

Merging #9314 (18130fa) into master (8997074) will increase coverage by 0.02%.
The diff coverage is 74.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9314      +/-   ##
==========================================
+ Coverage   60.37%   60.39%   +0.02%     
==========================================
  Files         585      587       +2     
  Lines       36851    36914      +63     
==========================================
+ Hits        22247    22294      +47     
- Misses      12630    12645      +15     
- Partials     1974     1975       +1     
Impacted Files Coverage Δ
server/rosetta.go 0.00% <ø> (ø)
server/rosetta/client_offline.go 0.00% <ø> (ø)
server/rosetta/client_online.go 0.00% <ø> (ø)
server/rosetta/config.go 0.00% <ø> (ø)
server/rosetta/util.go 0.00% <ø> (ø)
x/authz/simulation/operations.go 77.64% <ø> (ø)
x/feegrant/filtered_fee.go 0.00% <ø> (ø)
server/rosetta/converter.go 55.95% <66.66%> (ø)
server/rosetta/lib/errors/errors.go 66.66% <66.66%> (ø)
server/rosetta/lib/errors/registry.go 100.00% <100.00%> (ø)
... and 2 more

@noandrea noandrea requested a review from fdymylja May 12, 2021 14:50
@noandrea noandrea marked this pull request as ready for review May 12, 2021 14:57
@lgtm-com
Copy link

lgtm-com bot commented May 12, 2021

This pull request introduces 1 alert when merging 3dd84f0 into b4125d1 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to field

@lgtm-com
Copy link

lgtm-com bot commented May 14, 2021

This pull request introduces 1 alert when merging ab2298f2588680bfa5f854ec7225ba5dae7423f8 into eb7d939 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to field

go.sum Show resolved Hide resolved
Copy link
Collaborator

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

utACK

@tac0turtle tac0turtle changed the title Noandrea/gh9300 rosetta rosetta: upgrade to newest version May 14, 2021
@lgtm-com
Copy link

lgtm-com bot commented May 14, 2021

This pull request introduces 1 alert when merging ef0d7cdd20d5fd3e94a6793003241944a76adbf4 into eb7d939 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to field

CHANGELOG.md Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented May 14, 2021

This pull request introduces 1 alert when merging 2ec06d2aac2c24a6336771b1597eccb54393e933 into eb7d939 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to field

@tac0turtle
Copy link
Member

Thanks for the PR, before we can go ahead and merge this could you touch on whom the maintenance of this code will fall?

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.

Looks good. This is not covered by simulation tests we are doing and only changing the rosetta server (except few autoformatting). So if you are going to handle it on top of the latest beta, then we can probably merge it in 0.43. @aaronc - what do you think?

@aaronc
Copy link
Member

aaronc commented May 14, 2021

Looks good. This is not covered by simulation tests we are doing and only changing the rosetta server (except few autoformatting). So if you are going to handle it on top of the latest beta, then we can probably merge it in 0.43. @aaronc - what do you think?

I think that's fine. Merge when ready.

@noandrea noandrea merged commit 25ecec6 into master May 14, 2021
@noandrea noandrea deleted the noandrea/gh9300-rosetta branch May 14, 2021 22:19
roysc pushed a commit to vulcanize/cosmos-sdk that referenced this pull request Jun 23, 2021
* feat:  update rosetta sdk to v0.6.10 

embed from v1.0.0 release branch of the library: https://github.com/tendermint/cosmos-rosetta-gateway/tree/release/v1.0.0

closes:
cosmos#9300

Co-authored-by: Alessio Treglia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Rosetta Issues and PR related to Rosetta C:Simulations C:x/authz C:x/feegrant T: ADR An issue or PR relating to an architectural decision record T:Docs Changes and features related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embed Coinbase Rosetta within the Cosmos SDK
9 participants