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

wallet: set nLockTime to the tip for withdrawal transactions #3465

Merged
merged 3 commits into from
Feb 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions bitcoin/tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,8 @@ static void bitcoin_tx_destroy(struct bitcoin_tx *tx)

struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
const struct chainparams *chainparams,
varint_t input_count, varint_t output_count)
varint_t input_count, varint_t output_count,
u32 nlocktime)
{
struct bitcoin_tx *tx = tal(ctx, struct bitcoin_tx);
assert(chainparams);
Expand All @@ -396,7 +397,7 @@ struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
tal_add_destructor(tx, bitcoin_tx_destroy);

tx->input_amounts = tal_arrz(tx, struct amount_sat*, input_count);
tx->wtx->locktime = 0;
tx->wtx->locktime = nlocktime;
tx->wtx->version = 2;
tx->chainparams = chainparams;
return tx;
Expand Down
3 changes: 2 additions & 1 deletion bitcoin/tx.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ size_t bitcoin_tx_weight(const struct bitcoin_tx *tx);
* zeroed with inputs' sequence_number set to FFFFFFFF) */
struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
const struct chainparams *chainparams,
varint_t input_count, varint_t output_count);
varint_t input_count, varint_t output_count,
u32 nlocktime);

/* This takes a raw bitcoin tx in hex. */
struct bitcoin_tx *bitcoin_tx_from_hex(const tal_t *ctx, const char *hex,
Expand Down
2 changes: 1 addition & 1 deletion channeld/commit_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx,
#endif

/* Worst-case sizing: both to-local and to-remote outputs. */
tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2);
tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2, 0);

/* We keep track of which outputs have which HTLCs */
*htlcmap = tal_arr(tx, const struct htlc *, tx->wtx->outputs_allocation_len);
Expand Down
2 changes: 1 addition & 1 deletion common/close_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx,
* * txin count: 1
*/
/* Now create close tx: one input, two outputs. */
tx = bitcoin_tx(ctx, chainparams, 1, 2);
tx = bitcoin_tx(ctx, chainparams, 1, 2, 0);

/* Our input spends the anchor tx output. */
bitcoin_tx_add_input(tx, anchor_txid, anchor_index,
Expand Down
3 changes: 2 additions & 1 deletion common/funding_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ struct bitcoin_tx *funding_tx(const tal_t *ctx,
struct bitcoin_tx *tx;
bool has_change = !amount_sat_eq(change, AMOUNT_SAT(0));

tx = tx_spending_utxos(ctx, chainparams, utxomap, bip32_base, has_change, 1);
tx = tx_spending_utxos(ctx, chainparams, utxomap, bip32_base,
has_change, 1, 0, BITCOIN_TX_DEFAULT_SEQUENCE);


wscript = bitcoin_redeem_2of2(tx, local_fundingkey, remote_fundingkey);
Expand Down
10 changes: 4 additions & 6 deletions common/htlc_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx,
struct amount_sat htlc_fee,
u32 locktime)
{
struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, 1);
/* BOLT #3:
* * locktime: `0` for HTLC-success, `cltv_expiry` for HTLC-timeout
*/
struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, 1, locktime);
u8 *wscript;
struct amount_sat amount;

Expand All @@ -34,11 +37,6 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx,
*/
assert(tx->wtx->version == 2);

/* BOLT #3:
* * locktime: `0` for HTLC-success, `cltv_expiry` for HTLC-timeout
*/
tx->wtx->locktime = locktime;

/* BOLT #3:
* * txin count: 1
* * `txin[0]` outpoint: `txid` of the commitment transaction and
Expand Down
2 changes: 1 addition & 1 deletion common/initial_commit_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx,


/* Worst-case sizing: both to-local and to-remote outputs. */
tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2);
tx = bitcoin_tx(ctx, chainparams, 1, untrimmed + 2, 0);

/* This could be done in a single loop, but we follow the BOLT
* literally to make comments in test vectors clearer. */
Expand Down
10 changes: 6 additions & 4 deletions common/utxo.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,17 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
const struct utxo **utxos,
const struct ext_key *bip32_base,
bool add_change_output,
size_t num_output)
size_t num_output,
u32 nlocktime,
u32 nsequence)
{
struct pubkey key;
u8 *script;

assert(num_output);
size_t outcount = add_change_output ? 1 + num_output : num_output;
struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, tal_count(utxos), outcount);
struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, tal_count(utxos),
outcount, nlocktime);

for (size_t i = 0; i < tal_count(utxos); i++) {
if (utxos[i]->is_p2sh && bip32_base) {
Expand All @@ -80,8 +83,7 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
}

bitcoin_tx_add_input(tx, &utxos[i]->txid, utxos[i]->outnum,
BITCOIN_TX_DEFAULT_SEQUENCE,
utxos[i]->amount, script);
nsequence, utxos[i]->amount, script);
}

return tx;
Expand Down
4 changes: 3 additions & 1 deletion common/utxo.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx,
const struct utxo **utxos,
const struct ext_key *bip32_base,
bool add_change_output,
size_t num_output);
size_t num_output,
u32 nlocktime,
u32 nsequence);

#endif /* LIGHTNING_COMMON_UTXO_H */
5 changes: 3 additions & 2 deletions common/withdraw_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
const struct pubkey *changekey,
struct amount_sat change,
const struct ext_key *bip32_base,
int *change_outnum)
int *change_outnum, u32 nlocktime)
{
struct bitcoin_tx *tx;
int output_count;

tx = tx_spending_utxos(ctx, chainparams, utxos, bip32_base,
!amount_sat_eq(change, AMOUNT_SAT(0)),
tal_count(outputs));
tal_count(outputs), nlocktime,
BITCOIN_TX_DEFAULT_SEQUENCE - 1);

output_count = bitcoin_tx_add_multi_outputs(tx, outputs);
assert(output_count == tal_count(outputs));
Expand Down
3 changes: 2 additions & 1 deletion common/withdraw_tx.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct utxo;
* @change: (in) amount to send as change.
* @bip32_base: (in) bip32 base for key derivation, or NULL.
* @change_outnum: (out) set to output index of change output or -1 if none, unless NULL.
* @nlocktime: (in) the value to set as the transaction's nLockTime.
*/
struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
const struct chainparams *chainparams,
Expand All @@ -33,6 +34,6 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx,
const struct pubkey *changekey,
struct amount_sat change,
const struct ext_key *bip32_base,
int *change_outnum);
int *change_outnum, u32 nlocktime);

#endif /* LIGHTNING_COMMON_WITHDRAW_TX_H */
2 changes: 1 addition & 1 deletion devtools/mkclose.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ int main(int argc, char *argv[])
|| !pubkey_from_privkey(&funding_privkey[REMOTE], &funding_pubkey[REMOTE]))
errx(1, "Bad deriving funding pubkeys");

tx = bitcoin_tx(NULL, chainparams, 1, 2);
tx = bitcoin_tx(NULL, chainparams, 1, 2, 0);

/* Our input spends the anchor tx output. */
bitcoin_tx_add_input(tx, &funding_txid, funding_outnum,
Expand Down
1 change: 1 addition & 0 deletions hsmd/hsm_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ msgdata,hsm_sign_withdrawal,num_outputs,u16,
msgdata,hsm_sign_withdrawal,outputs,bitcoin_tx_output,num_outputs
msgdata,hsm_sign_withdrawal,num_inputs,u16,
msgdata,hsm_sign_withdrawal,inputs,utxo,num_inputs
msgdata,hsm_sign_withdrawal,nlocktime,u32,

msgtype,hsm_sign_withdrawal_reply,107
msgdata,hsm_sign_withdrawal_reply,tx,bitcoin_tx,
Expand Down
5 changes: 3 additions & 2 deletions hsmd/hsmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1643,10 +1643,11 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
struct bitcoin_tx *tx;
struct pubkey changekey;
struct bitcoin_tx_output **outputs;
u32 nlocktime;

if (!fromwire_hsm_sign_withdrawal(tmpctx, msg_in, &satoshi_out,
&change_out, &change_keyindex,
&outputs, &utxos))
&outputs, &utxos, &nlocktime))
return bad_req(conn, c, msg_in);

if (!bip32_pubkey(&secretstuff.bip32, &changekey, change_keyindex))
Expand All @@ -1655,7 +1656,7 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,

tx = withdraw_tx(tmpctx, c->chainparams,
cast_const2(const struct utxo **, utxos), outputs,
&changekey, change_out, NULL, NULL);
&changekey, change_out, NULL, NULL, nlocktime);

sign_all_inputs(tx, utxos);

Expand Down
3 changes: 1 addition & 2 deletions onchaind/onchaind.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,7 @@ static struct bitcoin_tx *tx_to_us(const tal_t *ctx,
u8 *msg;
u8 **witness;

tx = bitcoin_tx(ctx, out->chainparams, 1, 1);
tx->wtx->locktime = locktime;
tx = bitcoin_tx(ctx, out->chainparams, 1, 1, locktime);
bitcoin_tx_add_input(tx, &out->txid, out->outnum, to_self_delay,
out->sat, NULL);

Expand Down
18 changes: 16 additions & 2 deletions wallet/walletrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ static struct command_result *broadcast_and_wait(struct command *cmd,
utx->wtx->change_key_index,
cast_const2(const struct bitcoin_tx_output **,
utx->outputs),
utx->wtx->utxos);
utx->wtx->utxos,
utx->tx->wtx->locktime);

if (!wire_sync_write(cmd->ld->hsm_fd, take(msg)))
fatal("Could not write sign_withdrawal to HSM: %s",
Expand Down Expand Up @@ -141,6 +142,7 @@ static struct command_result *json_prepare_tx(struct command *cmd,
const u8 *destination = NULL;
size_t out_len, i;
const struct utxo **chosen_utxos = NULL;
u32 locktime = 0;

*utx = tal(cmd, struct unreleased_tx);
(*utx)->wtx = tal(*utx, struct wallet_tx);
Expand Down Expand Up @@ -278,6 +280,17 @@ static struct command_result *json_prepare_tx(struct command *cmd,
p_opt("utxos", param_utxos, &chosen_utxos),
NULL))
return command_param_failed();
/* Setting the locktime to the next block to be mined has multiple
* benefits:
* - anti fee-snipping (even if not yet likely)
* - less distinguishable transactions (with this we create
* general-purpose transactions which looks like bitcoind:
* native segwit, nlocktime set to tip, and sequence set to
* 0xFFFFFFFE by default. Other wallets are likely to implement
* this too).
* FIXME: Do we want to also fuzz this like bitcoind does ?
*/
locktime = cmd->ld->topology->tip->height;
darosior marked this conversation as resolved.
Show resolved Hide resolved
}

if (!feerate_per_kw) {
Expand Down Expand Up @@ -397,7 +410,8 @@ static struct command_result *json_prepare_tx(struct command *cmd,
(*utx)->wtx->utxos, (*utx)->outputs,
changekey, (*utx)->wtx->change,
cmd->ld->wallet->bip32_base,
&(*utx)->change_outnum);
&(*utx)->change_outnum,
locktime);
bitcoin_txid((*utx)->tx, &(*utx)->txid);

return NULL;
Expand Down