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

Option to disable incoming push amounts on channel opening #2027

Merged

Conversation

@rodentrabies
Copy link
Contributor

@rodentrabies rodentrabies commented Oct 10, 2018

Implements proposal in #1884.

Progress:

  • config option and check implementation;
  • test.

Fixes #1884.

@rodentrabies rodentrabies force-pushed the no-incoming-push-amounts-option branch from 5f8e807 to 975e9a3 Oct 10, 2018
config.go Outdated Show resolved Hide resolved
server.go Outdated Show resolved Hide resolved
@rodentrabies rodentrabies force-pushed the no-incoming-push-amounts-option branch from 975e9a3 to b124190 Oct 11, 2018
@rodentrabies
Copy link
Contributor Author

@rodentrabies rodentrabies commented Oct 11, 2018

I'm not quite sure about adding another error type (lnwire.ErrNonZeroPushAmount), but it seems ok.

Copy link
Collaborator

@wpaulino wpaulino left a comment

LGTM ⚡️

lnwire/error.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@halseth halseth left a comment

LGTM 👍

lnwallet/errors.go Outdated Show resolved Hide resolved
fundingmanager_test.go Show resolved Hide resolved
rodentrabies added 2 commits Nov 1, 2018
This is useful for merchant-side prevention of accidental pushes
during channel opening.
@rodentrabies rodentrabies force-pushed the no-incoming-push-amounts-option branch from 3a8e738 to 6f3adcd Nov 1, 2018
@Roasbeef Roasbeef merged commit f60012b into lightningnetwork:master Nov 3, 2018
1 of 2 checks passed
@rodentrabies rodentrabies deleted the no-incoming-push-amounts-option branch Nov 4, 2018
cfg.RejectPush = true
defer func() {
tearDownFundingManagers(t, alice, bob)
cfg.RejectPush = rejectPush
Copy link
Collaborator

@cfromknecht cfromknecht Nov 11, 2018

just leaving a note that this could be lead to a race condition in the unit tests if other tests are reading from cfg in parallel. we can revisit if it surfaces

Copy link
Collaborator

@halseth halseth Nov 12, 2018

Hm yeah, value should probably be copied to the fundingMgr config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants