Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

rpc: refactor rpc packages and backend to support cosmos namespace #1070

Merged
merged 5 commits into from May 2, 2022

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Apr 30, 2022

Description

This PR refactors the RPC package so that it's easier to implement the "cosmos" namespace per Wallet Connect V2. See Changelog for a detailed list of API breaking changes


For contributor use:

  • 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

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@github-actions github-actions bot added C:CLI Type: Tests issues and PR related to tests labels Apr 30, 2022
@semgrep-app
Copy link

semgrep-app bot commented Apr 30, 2022

Semgrep found 1 hidden-goroutine finding:

  • rpc/namespaces/ethereum/eth/filters/subscription.go: L35-52

Detected a hidden goroutine. Function invocations are expected to synchronous, and this function will execute asynchronously because all it does is call a goroutine. Instead, remove the internal goroutine and call the function using 'go'.

⚪️ This finding does not block your pull request.
🙈 From go.lang.best-practice.hidden-goroutine.hidden-goroutine.

Copy link
Contributor

@loredanacirstea loredanacirstea left a comment

Choose a reason for hiding this comment

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

utACK
I understand that this PR prepares the cosmos namespace to be added on the same level as the Ethereum namespaces.

rpc/backend/backend.go Outdated Show resolved Hide resolved
Copy link
Contributor

@crypto-facs crypto-facs left a comment

Choose a reason for hiding this comment

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

LGTM

@fedekunze fedekunze enabled auto-merge (squash) May 2, 2022 06:12
Co-authored-by: Loredana Cirstea <loredana.cirstea@gmail.com>
@fedekunze fedekunze merged commit c25669c into main May 2, 2022
@fedekunze fedekunze deleted the fedekunze/refactor-rpc branch May 2, 2022 06:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C:CLI Type: Tests issues and PR related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants