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

mfc: bug fixes for v2 opens #4294

Merged
merged 4 commits into from
Jan 6, 2021

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Dec 21, 2020

Fixes a few problems found while running tests against the v2 open pathways.

Update: this now depends on #4295, as we use the take() semantic for json_add_psbt in this patchset. First 'new commit' is 6cb23fb

Changelog-None.

@cdecker
Copy link
Member

cdecker commented Dec 23, 2020

Looking good, but need to settle #4295 first.

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack, modulo the weird rounding change.

contrib/pyln-client/pyln/client/lightning.py Outdated Show resolved Hide resolved
Ideally we'd 'cure' the error and re-attempt, except that if this was a
bitcoin-backend 'failure to broadcast' then it really needs user
intervention to figure out what's wrong -- it's possible that the
peer successfully broadcast the transaction
We only want to attempt to unreserve inputs that are ours (otherwise we
log a broken error)
Since we round down in `amount_tx_fee`, find the change fee as the
difference between what we've already paid and what the combined/total
fee would be if the change weight were also added.
This will cause blow ups for v2 multifundchannel attempts with failures,
but allows us to return the expected errors for single-shot
fundchannel attempts.

Error handling is coming, i promise
@cdecker
Copy link
Member

cdecker commented Jan 6, 2021

Rebased on top of #4295, ready to go.

ACK e1938cd

@cdecker cdecker merged commit 0726d29 into ElementsProject:master Jan 6, 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