Skip to content

Commit

Permalink
no need for confirmation in the device for change address ref #139
Browse files Browse the repository at this point in the history
  • Loading branch information
stdevAlDen committed Dec 11, 2019
1 parent 71ab1a5 commit eb42972
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 39 deletions.
33 changes: 26 additions & 7 deletions src/coin/skycoin/models/testsutils.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package skycoin

import (
"github.com/davecgh/go-spew/spew"
"github.com/fibercrypto/fibercryptowallet/src/coin/skycoin/testsuite"
"github.com/fibercrypto/fibercryptowallet/src/core"
"github.com/fibercrypto/fibercryptowallet/src/util"
Expand Down Expand Up @@ -101,7 +100,7 @@ func (tOpt *TransferOptions) SetValue(key string, value interface{}) {
}

// GenerateTestKeyPair provides deterministic sequence of test keys
//// that can be recovered later inside a wallet
// that can be recovered later inside a wallet
func GenerateTestKeyPair(t *testing.T) (*KeyData, error) {
var err error
if seedEntropy == nil {
Expand Down Expand Up @@ -143,9 +142,9 @@ func GenerateTestKeyPair(t *testing.T) (*KeyData, error) {
var global_mock *SkycoinApiMock

var logModelTest = logging.MustGetLogger("Skycoin Model Test")
// CleanGlobalMock util when is needed to change the values of an

// API method used in other test with different values.

// CleanGlobalMock util when is needed to change the values of an
func CleanGlobalMock() {
if global_mock == nil {
global_mock = new(SkycoinApiMock)
Expand All @@ -154,7 +153,18 @@ func CleanGlobalMock() {
}
}

func TransactionSignInputTestImpl(t *testing.T, signer core.TxnSigner) {
func TransactionSignInputTestSkyHwImpl(t *testing.T, signers []core.TxnSigner, setWallet func(*testing.T, core.TxnSigner, core.Wallet)) {
_, keysData, _, err := makeTransactionMultipleInputs(t, 3)
require.NoError(t, err)
// Load local wallets
wallets := makeLocalWalletsFromKeyData(t, keysData)
for idx := range signers {
setWallet(t, signers[idx], wallets[idx])
}
TransactionSignInputTestImpl(t, signers)
}

func TransactionSignInputTestImpl(t *testing.T, signers []core.TxnSigner) {
txn, keysData, uxspent, err := makeTransactionMultipleInputs(t, 3)
require.NoError(t, err)
// Mock UxOut API calls
Expand All @@ -171,12 +181,16 @@ func TransactionSignInputTestImpl(t *testing.T, signer core.TxnSigner) {

// Load local wallets
wallets := makeLocalWalletsFromKeyData(t, keysData)
if signer != nil {
for _, signer := range signers {
err = util.AttachSignService(signer)
require.NoError(t, err)
}

// Input is already signed
var signer core.TxnSigner
if len(signers) > 0 {
signer = signers[0]
}
_, err = wallets[0].Sign(uiTxn, signer, util.EmptyPassword, []string{"0"})
testutil.RequireError(t, err, "Transaction is fully signed")
isFullySigned, err = uiTxn.IsFullySigned()
Expand All @@ -188,6 +202,9 @@ func TransactionSignInputTestImpl(t *testing.T, signer core.TxnSigner) {
isFullySigned, err = uiTxn.IsFullySigned()
require.NoError(t, err)
require.False(t, isFullySigned)
if len(signers) > 1 {
signer = signers[1]
}
signedCoreTxn, err = wallets[1].Sign(uiTxn, signer, nil, []string{"1"})
require.NoError(t, err)
signedTxn, isUninjected := signedCoreTxn.(*SkycoinUninjectedTransaction)
Expand All @@ -199,6 +216,9 @@ func TransactionSignInputTestImpl(t *testing.T, signer core.TxnSigner) {
isFullySigned, err = signedTxn.IsFullySigned()
require.NoError(t, err)
require.True(t, isFullySigned)
if len(signers) > 2 {
signer = signers[2]
}
_, err = wallets[1].Sign(signedTxn, signer, nil, []string{"1"})
// FIXME use a named var from errors
testutil.RequireError(t, err, "Transaction is fully signed")
Expand All @@ -208,7 +228,6 @@ func TransactionSignInputTestImpl(t *testing.T, signer core.TxnSigner) {
isFullySigned, err = uiTxn.IsFullySigned()
require.NoError(t, err)
require.False(t, isFullySigned)
spew.Dump("{{{{{uiTxn", uiTxn)
signedCoreTxn, err = wallets[2].Sign(uiTxn, signer, nil, []string{"2"})
require.NoError(t, err)
signedTxn, isUninjected = signedCoreTxn.(*SkycoinUninjectedTransaction)
Expand Down
39 changes: 22 additions & 17 deletions src/contrib/skywallet/sky-wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
var logSkyWallet = logging.MustGetLogger("Skycoin hardware wallet")

type SkyWallet struct {
wlt core.Wallet
dev skyWallet.Devicer
handleButtonAckSequence func(dev skywallet.Devicer, prvMsg wire.Message, outsLen int) (wire.Message, error)
}
Expand Down Expand Up @@ -94,8 +95,9 @@ func (sw SkyWallet) ReadyForTxn(wlt core.Wallet, txn core.Transaction) (bool, er
}

// NewSkyWallet create a new sky wallet instance
func NewSkyWallet(dev skyWallet.Devicer, buttonAckHandler func(dev skywallet.Devicer, prvMsg wire.Message, outsLen int) (wire.Message, error)) *SkyWallet {
func NewSkyWallet(wlt core.Wallet, dev skyWallet.Devicer, buttonAckHandler func(dev skywallet.Devicer, prvMsg wire.Message, outsLen int) (wire.Message, error)) *SkyWallet {
return &SkyWallet{
wlt: wlt,
dev: dev,
handleButtonAckSequence: buttonAckHandler,
}
Expand Down Expand Up @@ -131,26 +133,29 @@ func getInputs(txn coin.Transaction, indexes []int) (inputs []*messages.SkycoinT
return inputs, nil
}

func getOutputs(tr coin.Transaction) (inputs []*messages.SkycoinTransactionOutput, err error) {
func getAddrIndex(wlt core.Wallet, addr string) (uint32, error) {
addrs, err := wlt.GetLoadedAddresses()
if err != nil {
return 0, err
}
for i := uint32(0); addrs.Next(); i++ {
if addrs.Value().String() == addr {
return i, nil
}
}
return 0, errors.New("unable to find a matching address from wallet")
}

func getOutputs(wlt core.Wallet, tr coin.Transaction) (inputs []*messages.SkycoinTransactionOutput, err error) {
for _, output := range tr.Out {
var transactionOutput messages.SkycoinTransactionOutput
transactionOutput.Address = proto.String(output.Address.String())
transactionOutput.Coin = proto.Uint64(output.Coins)
transactionOutput.Hour = proto.Uint64(output.Hours)
// FIXME: should be possible to send the address index
// find index to check if it is determined as a owned address
// transactionOutput.AddressIndex = proto.Uint32(uint32(addressIndex[i]))
// check out hw implementation
//addrIndex, err := strconv.Atoi(s[i])
//if err != nil {
// logSkyWallet.WithField("str_addr_index", s[i]).Error("unable to get integer from string")
// return nil, errors.New("error getting address index as integer")
//}
//if addrIndex < 0 {
// logSkyWallet.WithField("addr_index", addrIndex).Error("addrIndex should be greater than 0")
// return nil, errors.New("addrIndex should be greater than 0")
// }
//transactionOutput.AddressIndex = proto.Uint32(uint32(addrIndex))
index, err := getAddrIndex(wlt, output.Address.String())
if err == nil {
transactionOutput.AddressIndex = proto.Uint32(index)
}
inputs = append(inputs, &transactionOutput)
}
return
Expand Down Expand Up @@ -219,7 +224,7 @@ func (sw *SkyWallet) signTxn(txn *coin.Transaction, idxs []int, dt string) (*coi
logSkyWallet.WithError(err).Errorln("unable to get inputs")
return nil, fce.ErrTxnSignFailure
}
transactionOutputs, err := getOutputs(*txn)
transactionOutputs, err := getOutputs(sw.wlt, *txn)
if err != nil {
logSkyWallet.WithError(err).Errorln("unable to get outputs")
return nil, fce.ErrTxnSignFailure
Expand Down
18 changes: 9 additions & 9 deletions src/contrib/skywallet/sky-wallet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestGetSignerUIDShouldBeOk(t *testing.T) {
Data: fb,
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerUID()
Expand All @@ -42,7 +42,7 @@ func TestGetSignerUIDShouldBeOk(t *testing.T) {

func TestGetSignerUIDShouldFailForUninitializedDevice(t *testing.T) {
// Giving
sw := NewSkyWallet(nil, callback)
sw := NewSkyWallet(nil, nil, callback)

// When
devId, err := sw.GetSignerUID()
Expand All @@ -57,7 +57,7 @@ func TestGetSignerUIDShouldFailOnDeviceError(t *testing.T) {
// Giving
dev := mocks.Devicer{}
dev.On("GetFeatures").Return(wire.Message{}, errors.New(""))
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerUID()
Expand All @@ -78,7 +78,7 @@ func TestGetSignerUIDShouldFailForFailResponse(t *testing.T) {
Data: fb,
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerUID()
Expand All @@ -95,7 +95,7 @@ func TestGetSignerUIDShouldFailForInvalidMessageType(t *testing.T) {
Kind: uint16(messages.MessageType_MessageType_PinMatrixAck),
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerUID()
Expand All @@ -119,7 +119,7 @@ func TestGetSignerDescriptionShouldBeOk(t *testing.T) {
Data: fb,
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devDescription, err := sw.GetSignerDescription()
Expand All @@ -145,7 +145,7 @@ func TestGetSignerDescriptionShouldFailOnDeviceError(t *testing.T) {
// Giving
dev := mocks.Devicer{}
dev.On("GetFeatures").Return(wire.Message{}, errors.New(""))
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerDescription()
Expand All @@ -166,7 +166,7 @@ func TestGetSignerDescriptionShouldFailForFailResponse(t *testing.T) {
Data: fb,
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerDescription()
Expand All @@ -183,7 +183,7 @@ func TestGetSignerDescriptionShouldFailForInvalidMessageType(t *testing.T) {
Kind: uint16(messages.MessageType_MessageType_PinMatrixAck),
}
dev.On("GetFeatures").Return(msg, nil)
sw := NewSkyWallet(&dev, callback)
sw := NewSkyWallet(nil, &dev, callback)

// When
devId, err := sw.GetSignerDescription()
Expand Down
16 changes: 14 additions & 2 deletions src/contrib/skywallet/skywallet_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package hardware

import (
skycoin "github.com/fibercrypto/fibercryptowallet/src/coin/skycoin/models"
"github.com/fibercrypto/fibercryptowallet/src/core"
"github.com/fibercrypto/fibercryptowallet/src/util"
integrationtestutil "github.com/fibercrypto/fibercryptowallet/test/integration/util"
"github.com/fibercrypto/skywallet-go/src/skywallet"
Expand Down Expand Up @@ -36,6 +37,17 @@ func TestTransactionSignInputFromDevice(t *testing.T) {
return msg, nil
}
dev := skywallet.NewDevice(skywallet.DeviceTypeEmulator)
hs := NewSkyWallet(dev, callback)
skycoin.TransactionSignInputTestImpl(t, hs)
hs1 := NewSkyWallet(nil, dev, callback)
hs2 := NewSkyWallet(nil, dev, callback)
hs3 := NewSkyWallet(nil, dev, callback)
setWallet := func(t *testing.T, signer core.TxnSigner, wlt core.Wallet) {
hs, ok := signer.(*SkyWallet)
require.True(t, ok)
hs.wlt = wlt
}
signers := make([]core.TxnSigner, 3)
signers[0] = hs1
signers[1] = hs2
signers[2] = hs3
skycoin.TransactionSignInputTestSkyHwImpl(t, signers, setWallet)
}
2 changes: 2 additions & 0 deletions src/errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ var (
ErrInvalidAddressString = errors.New("Error decoding base58 address")
// ErrTxnSignFailure signing strategy reported an error whie signing transaction
ErrTxnSignFailure = errors.New("Transaction signing failed for txn")
// ErrNoMoreElements no more elements in a sequence or succession
ErrNoMoreElements = errors.New("No more elements in the sequence")
// ErrUnexpectedUxOutAddress unexpected address
ErrUnexpectedUxOutAddress = errors.New("Unexpected address")
// ErrInvalidPoolObjectType clients in the pool do not match expected type
Expand Down
15 changes: 11 additions & 4 deletions src/models/walletsModel.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/sirupsen/logrus"
wlcore "github.com/fibercrypto/fibercryptowallet/src/main"
messages "github.com/fibercrypto/skywallet-protob/go"
fccore "github.com/fibercrypto/fibercryptowallet/src/core"
)

const (
Expand Down Expand Up @@ -82,7 +83,7 @@ func (walletModel *WalletModel) init() {
}

// attachHwAsSigner add a hw as signer
func attachHwAsSigner(dev skyWallet.Devicer) error {
func attachHwAsSigner(wlt fccore.Wallet, dev skyWallet.Devicer) error {
pb := func(dev skyWallet.Devicer, prvMsg wire.Message, nextMsg messages.MessageType) (wire.Message, error) {
msg, err := dev.ButtonAck()
if err != nil {
Expand All @@ -100,6 +101,9 @@ func attachHwAsSigner(dev skyWallet.Devicer) error {
if str == "Action cancelled by user" {
return msg, fce.ErrHwSignTransactionCanceled
}
} else if msg.Kind == uint16(messages.MessageType_MessageType_ResponseTransactionSign) {
logSignersModel.Warningln("some addresses was not confirmed in the device because are owned for this wallet")
return msg, fce.ErrNoMoreElements
}
logSignersModel.WithFields(
logrus.Fields{
Expand Down Expand Up @@ -133,13 +137,16 @@ func attachHwAsSigner(dev skyWallet.Devicer) error {
msg, err = pb(dev, prvMsg, messages.MessageType_MessageType_ButtonRequest)
}
if err != nil {
if err == fce.ErrNoMoreElements {
return msg, nil
}
return wire.Message{}, err
}
outsLen--
}
return msg, nil
}
hw := hardware.NewSkyWallet(dev, cb)
hw := hardware.NewSkyWallet(wlt, dev, cb)
am := wlcore.LoadAltcoinManager()
if err := am.AttachSignService(hw); err != nil {
logSignersModel.Errorln("error registering hardware wallet as signer")
Expand All @@ -150,7 +157,7 @@ func attachHwAsSigner(dev skyWallet.Devicer) error {

// sniffHw notify the model about available hardware wallet device if any
func (walletModel *WalletModel) sniffHw() {
dev := skyWallet.NewDevice(skyWallet.DeviceTypeUSB)
dev := skyWallet.NewDevice(skyWallet.DeviceTypeEmulator)
addr, err := hardware.HwFirstAddr(dev)
if err == nil {
wlt, err := walletManager.WalletEnv.LookupWallet(addr)
Expand All @@ -159,7 +166,7 @@ func (walletModel *WalletModel) sniffHw() {
// FIXME handle this scenario with a wallet registration.
return
}
err = attachHwAsSigner(dev)
err = attachHwAsSigner(wlt, dev)
if err != nil {
logSignersModel.WithError(err).Errorln("unable to attach signer")
return
Expand Down

0 comments on commit eb42972

Please sign in to comment.