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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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 •
edited
Loading

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

tomtau
tomtau reviewed May 14, 2021
View reviewed changes
go.sum Show resolved Hide resolved
fedekunze
fedekunze approved these changes May 14, 2021
View reviewed changes
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

server/rosetta/lib/internal/service/construction.go Show resolved Hide resolved
server/rosetta/lib/internal/service/data.go Show resolved Hide resolved
@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

alessio
alessio reviewed May 14, 2021
View reviewed changes
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?

robert-zaremba
robert-zaremba approved these changes May 14, 2021
View reviewed changes
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 •
edited
Loading

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 and others added 12 commits May 14, 2021 18:38
@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
@noandrea @roysc
rosetta: upgrade to newest version (cosmos#9314)
01ab3d9 
* 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 <alessio@tendermint.com>
@noandrea noandrea mentioned this pull request Jun 25, 2021
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomtau tomtau tomtau left review comments

@alessio alessio alessio left review comments

@jgimeno jgimeno jgimeno approved these changes

@robert-zaremba robert-zaremba robert-zaremba approved these changes

@fdymylja fdymylja Awaiting requested review from fdymylja

@aaronc aaronc Awaiting requested review from aaronc

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

@fedekunze fedekunze Awaiting requested review from fedekunze

@amaury1093 amaury1093 Awaiting requested review from amaury1093

@tac0turtle tac0turtle Awaiting requested review from tac0turtle

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Embed Coinbase Rosetta within the Cosmos SDK
9 participants
@noandrea @tac0turtle @aaronc @alessio @robert-zaremba @tomtau @jgimeno @fedekunze @fdymylja