-
Notifications
You must be signed in to change notification settings - Fork 92
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
Build improvements #890
Build improvements #890
Conversation
This is a preparation for bumping the lndclient dependency. Because each lndclient service mock now needs to implement the RawClientWithMacAuth method but with a different type, we can't implement two lndclient service mocks within a single struct, as that would require us to have two methods with the same name but different types. So we split the lnd and rounter mocks into two separate structs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 🙏
case "DevRPC": | ||
return &devrpc.Config{ | ||
ActiveNetParams: &chaincfg.RegressionNetParams, | ||
GraphDB: &channeldb.ChannelGraph{}, | ||
}, true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@ViktorTigerstrom: review reminder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this @guggero 🚀!
Leaving 1 question and 1 nit 😃
@@ -94,7 +94,7 @@ DOCKER_TOOLS = docker run \ | |||
-v $(shell bash -c "mkdir -p /tmp/go-lint-cache; echo /tmp/go-lint-cache"):/root/.cache/golangci-lint \ | |||
-v $$(pwd):/build litd-tools | |||
|
|||
ITEST_TAGS := integration itest $(LND_RELEASE_TAGS) | |||
ITEST_TAGS := dev integration itest lowscrypt $(LND_RELEASE_TAGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'm not fully sure why the lowscrypt
buildtag is added here? Maybe the commit message could be extended to specify that as well if this is intended :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. Speeds up wallet creation by using weaker encryption. Added that to the commit message.
The dev flag allows us to enable some of lnd's configuration flags that are only available for testing. To speed up any wallet interaction, we also add the lowscrypt build tag that chooses weak encryption for the wallet that is much faster (a couple of milliseconds vs. multiple seconds per wallet creation).
6b2562b
to
f50fb91
Compare
This PR pull out some commits from #848 that don't have any dependency on
lnd v0.18.4
and can be merged immediately.Didn't add release notes as the improvements are mostly build/development related and don't include any end user features.