Skip to content
This repository has been archived by the owner on Nov 2, 2018. It is now read-only.

Add support for offline transaction signing #2907

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d39e94f
add offline signing functionality
lukechampine Mar 27, 2018
1c7efd4
sync before reporting wallet height
lukechampine Mar 27, 2018
4fa416a
add api routes for unspent+sign
lukechampine Mar 27, 2018
e959025
add api docs for unspent+sign
lukechampine Mar 27, 2018
6332d01
change sign semantics
lukechampine Mar 28, 2018
b3741e7
add wallet sign command
lukechampine Mar 28, 2018
41c5410
generate keys incrementally
lukechampine Mar 28, 2018
32059e7
use new SpendableOutput type for /unspent
lukechampine Mar 28, 2018
277d93a
sign SiafundInputs as well
lukechampine Mar 28, 2018
873500b
Add siatest and client integration for offline signing
ChrisSchinnerl Mar 29, 2018
043674b
Merge pull request #2913 from NebulousLabs/offline-signing-siatest
lukechampine Mar 29, 2018
a2bcb24
Merge branch 'master' into offline-signing
Mar 29, 2018
90566ab
decode directly into toSign map
lukechampine Mar 29, 2018
d408cc1
add docstrings
lukechampine Mar 29, 2018
4050676
document tosign types
lukechampine Mar 29, 2018
64ff690
account for unconfirmed txns in SpendableOutputs
lukechampine Apr 12, 2018
78c2a13
add wallet sign -raw flag, JSON by default
lukechampine Apr 17, 2018
d2c89fc
try /wallet/sign before doing keygen
lukechampine Apr 17, 2018
c5098c8
include UnlockConditions in SpendableOutput
lukechampine Apr 17, 2018
c1c14d7
Revert "include UnlockConditions in SpendableOutput"
lukechampine Apr 18, 2018
0514348
Merge branch 'master' into offline-signing
lukechampine May 14, 2018
6b22c87
don't include unconfirmed outputs that may be spent
lukechampine May 16, 2018
477a497
add UnlockConditions to SpendableOutput
lukechampine May 30, 2018
c36c14e
Merge branch 'master' into offline-signing
lukechampine May 30, 2018
30a5854
Merge branch 'master' into offline-signing
lukechampine Jul 11, 2018
83967e1
fix TransactionPoolRawPost signature
lukechampine Jul 12, 2018
4fccafd
add wallet broadcast cmd
lukechampine Jul 12, 2018
c97a9d7
more helpful signature decoding error
lukechampine Jul 12, 2018
f174e41
overhaul SignTransaction
lukechampine Jul 12, 2018
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
34 changes: 34 additions & 0 deletions modules/wallet/transactionbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package wallet
import (
"bytes"
"errors"
"math"
"sort"

"github.com/NebulousLabs/Sia/crypto"
Expand Down Expand Up @@ -642,6 +643,7 @@ func (w *Wallet) SpendableOutputs() []modules.SpendableOutput {
// ensure durability of reported outputs
w.syncDB()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to sync here? Once you got the outputs and created a transaction, the transaction pool will decide if the transaction is valid. Therefore I think it is not really an issue if the output disappears from the wallet due to a crash.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's more about the principle. If the wallet reports that it has an output, and then it crashes, it should still have that output next time you ask it. The general rule is: "don't give the user any data that won't survive a crash."

Copy link
Contributor

Choose a reason for hiding this comment

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

I see 👍


// build initial list of confirmed outputs
var outputs []modules.SpendableOutput
dbForEachSiacoinOutput(w.dbTx, func(scoid types.SiacoinOutputID, sco types.SiacoinOutput) {
outputs = append(outputs, modules.SpendableOutput{
Expand All @@ -660,6 +662,23 @@ func (w *Wallet) SpendableOutputs() []modules.SpendableOutput {
})
})

// don't include outputs marked as spent in pending transactions
Copy link
Member Author

Choose a reason for hiding this comment

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

@DavidVorick this seemed like a reasonable thing to do, but would like to hear your thoughts

pending := make(map[types.OutputID]struct{})
for _, pt := range w.unconfirmedProcessedTransactions {
for _, input := range pt.Inputs {
if input.WalletAddress {
pending[input.ParentID] = struct{}{}
}
}
}
filtered := outputs[:0]
for _, o := range outputs {
if _, ok := pending[o.ID]; !ok {
filtered = append(filtered, o)
}
}
outputs = filtered

// set the confirmation height for each output
outer:
for i, o := range outputs {
Expand All @@ -678,6 +697,21 @@ outer:
}
}

// add unconfirmed outputs
Copy link
Member Author

Choose a reason for hiding this comment

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

ditto here

for _, pt := range w.unconfirmedProcessedTransactions {
for _, o := range pt.Outputs {
if o.WalletAddress {
outputs = append(outputs, modules.SpendableOutput{
FundType: types.SpecifierSiacoinOutput,
ID: o.ID,
UnlockHash: o.RelatedAddress,
Value: o.Value,
ConfirmationHeight: types.BlockHeight(math.MaxUint64), // unconfirmed
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure what the best approach is here. I guess this is an argument for making the field Confirmations instead of ConfirmationHeight; then we could set Confirmations: 0. If we instead set ConfirmationHeight to some special value, the API client has to do special processing to calculate the number of confirmations.

})
}
}
}

return outputs
}

Expand Down