Skip to content

Conversation

carlaKC
Copy link
Contributor

@carlaKC carlaKC commented Dec 13, 2021

The looprpc package is used for client and server protos, despite their being in different directories (github.com/lightninglabs/loop/looprpc and github.com/lightninglabs/loop/swapserverrpc). This is required to maintain backwards compatibility, because the package is used as part of the URI in grpc.

[1] Use of mismatched package path/names (eg, import path github.com/lightninglabs/loop/swapserverrpc for looprpc package) causes issues when loop is imported as a package (as is the case in the server's itests) - the import can't be found.

[2] Likewise, the presence of two looprpc packages (as is the case in loopd/swapclient_server.go) also confuses modules when we import loopd (to run loopd as a package, rather than a standalone binary in itests).

This PR updates loop to alias swapserverrpc with looprpc everywhere to address [1] and uses distinct aliases in loopd to address [2].

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

Packages importing loop can't import this package without aliasing
the import.
Packages importing loopd can't import 2x looprpc packages (in the
form of swapserverrpc and looprpc). To get around this, we alias
each import with a different name, starting with the client in this
commit.
@carlaKC carlaKC requested review from guggero and bhandras December 14, 2021 08:04
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

clientrpc "github.com/lightninglabs/loop/looprpc"
"github.com/lightninglabs/loop/swap"
"github.com/lightninglabs/loop/swapserverrpc"
serverrpc "github.com/lightninglabs/loop/swapserverrpc"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use looprpc here to look the same as in the first two commits? Or use serverrpc there as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the server import might get upset with 2x looprpc packages and one aliased with looprpc, but just tried it out and it's fine. Will update!

Copy link
Member

@bhandras bhandras left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Packages importing loopd can't import 2x looprpc packages (in the
form of swapserverrpc and looprpc). To get around this, we alias
each import with a different name, updating the server in this
commit.
@carlaKC carlaKC merged commit c523789 into lightninglabs:master Dec 14, 2021
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.

3 participants