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

Build improvements #890

Merged
merged 4 commits into from
Nov 7, 2024
Merged

Build improvements #890

merged 4 commits into from
Nov 7, 2024

Conversation

guggero
Copy link
Member

@guggero guggero commented Oct 30, 2024

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.

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.
Copy link
Member

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

lgtm 🙏

Comment on lines +64 to +68
case "DevRPC":
return &devrpc.Config{
ActiveNetParams: &chaincfg.RegressionNetParams,
GraphDB: &channeldb.ChannelGraph{},
}, true
Copy link
Member

Choose a reason for hiding this comment

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

👍

@lightninglabs-deploy
Copy link

@ViktorTigerstrom: review reminder

Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a 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 😃

.github/workflows/main.yml Show resolved Hide resolved
@@ -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)
Copy link
Contributor

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 :)

Copy link
Member Author

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.

Makefile Outdated Show resolved Hide resolved
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).
@guggero guggero merged commit cb6977f into master Nov 7, 2024
12 of 13 checks passed
@guggero guggero deleted the random-improvements branch November 7, 2024 09:08
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.

4 participants