Skip to content

Commit

Permalink
upgrade multisig
Browse files Browse the repository at this point in the history
- update authentication to consider signature weights
- cleanup tests and code
- redo handlers tests

resolve #285
  • Loading branch information
husio committed Mar 21, 2019
1 parent afaeefc commit 53b295a
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 225 deletions.
76 changes: 35 additions & 41 deletions x/multisig/decorator.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func NewDecorator(auth x.Authenticator) Decorator {
// Check enforce multisig contract before calling down the stack
func (d Decorator) Check(ctx weave.Context, store weave.KVStore, tx weave.Tx, next weave.Checker) (weave.CheckResult, error) {
var res weave.CheckResult
newCtx, err := d.withMultisig(ctx, store, tx)
newCtx, err := d.authMultisig(ctx, store, tx)
if err != nil {
return res, err
}
Expand All @@ -33,62 +33,56 @@ func (d Decorator) Check(ctx weave.Context, store weave.KVStore, tx weave.Tx, ne
// Deliver enforces multisig contract before calling down the stack
func (d Decorator) Deliver(ctx weave.Context, store weave.KVStore, tx weave.Tx, next weave.Deliverer) (weave.DeliverResult, error) {
var res weave.DeliverResult
newCtx, err := d.withMultisig(ctx, store, tx)
newCtx, err := d.authMultisig(ctx, store, tx)
if err != nil {
return res, err
}

return next.Deliver(newCtx, store, tx)
}

func (d Decorator) withMultisig(ctx weave.Context, store weave.KVStore, tx weave.Tx) (weave.Context, error) {
if multisigContract, ok := tx.(MultiSigTx); ok {
ids := multisigContract.GetMultisig()
for _, contractID := range ids {
if contractID == nil {
return ctx, nil
}
func (d Decorator) authMultisig(ctx weave.Context, store weave.KVStore, tx weave.Tx) (weave.Context, error) {
multisigContract, ok := tx.(multiSigTx)
if !ok {
return ctx, nil
}

// check if we already have it
if d.auth.HasAddress(ctx, MultiSigCondition(contractID).Address()) {
return ctx, nil
}
ids := multisigContract.GetMultisig()
for _, contractID := range ids {
if contractID == nil {
return ctx, nil
}

// load contract
contract, err := d.getContract(store, contractID)
if err != nil {
return ctx, err
}
// If already authenticated it does not matter if multisig can
// authenticate as well. Any authentication method is enough.
if d.auth.HasAddress(ctx, MultiSigCondition(contractID).Address()) {
return ctx, nil
}

// collect all sigs
sigs := make([]weave.Address, len(contract.Participants))
for i, p := range contract.Participants {
sigs[i] = p.Signature
}
contract, err := d.bucket.GetContract(store, contractID)
if err != nil {
return ctx, err
}

// check sigs (can be sig or multisig)
authenticated := x.HasNAddresses(ctx, d.auth, sigs, int(contract.ActivationThreshold))
if !authenticated {
return ctx, errors.Wrapf(errors.ErrUnauthorized, "contract=%X", contractID)
var power Weight
for _, p := range contract.Participants {
if d.auth.HasAddress(ctx, p.Signature) {
power += p.Power
}

ctx = withMultisig(ctx, contractID)
}
if power < contract.ActivationThreshold {
return ctx, errors.Wrapf(errors.ErrUnauthorized, "%d power is not enough", power)
}

ctx = withMultisig(ctx, contractID)
}

return ctx, nil
}

func (d Decorator) getContract(store weave.KVStore, id []byte) (*Contract, error) {
obj, err := d.bucket.Get(store, id)
if err != nil {
return nil, err
}

if obj == nil || (obj != nil && obj.Value() == nil) {
return nil, errors.ErrNotFound
}

contract := obj.Value().(*Contract)
return contract, err
// multiSigTx is an optional interface for a Tx that allows it to
// support multisig contract. Multisig authentication can be done only
// for transactions that do support this interface.
type multiSigTx interface {
GetMultisig() [][]byte
}
29 changes: 11 additions & 18 deletions x/multisig/decorator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/iov-one/weave/store"
"github.com/iov-one/weave/weavetest"
"github.com/iov-one/weave/x"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -66,13 +65,13 @@ func TestDecorator(t *testing.T) {
tx weave.Tx
signers []weave.Condition
perms []weave.Condition
err error
wantErr *errors.Error
}{
"doesn't support multisig interface": {
"does not support multisig interface": {
tx: &weavetest.Tx{Msg: &weavetest.Msg{Serialized: []byte{1, 2, 3}}},
signers: []weave.Condition{a},
},
"Correct interface but no content": {
"correct interface but no content": {
tx: multisigTx([]byte("john"), nil),
signers: []weave.Condition{a},
},
Expand All @@ -84,12 +83,12 @@ func TestDecorator(t *testing.T) {
"with multisig contract but not enough signatures to activate": {
tx: multisigTx([]byte("foo"), contractID1),
signers: []weave.Condition{a},
err: errors.Wrapf(errors.ErrUnauthorized, "contract=%X", contractID1),
wantErr: errors.ErrUnauthorized,
},
"with invalid multisig contract ID": {
tx: multisigTx([]byte("foo"), []byte("bad id")),
signers: []weave.Condition{a, b},
err: errors.ErrNotFound,
wantErr: errors.ErrNotFound,
},
"contractID3 is activated by contractID2": {
tx: multisigTx([]byte("foo"), contractID2, contractID3),
Expand All @@ -105,7 +104,7 @@ func TestDecorator(t *testing.T) {
tx: multisigTx([]byte("foo"), contractID3),
// cconditions for ontractID2 are there but ontractID2 must be passed explicitly
signers: []weave.Condition{d, e},
err: errors.Wrapf(errors.ErrUnauthorized, "contract=%X", contractID3),
wantErr: errors.ErrUnauthorized,
},
}

Expand All @@ -120,19 +119,13 @@ func TestDecorator(t *testing.T) {
var hn MultisigCheckHandler
stack := weavetest.Decorate(&hn, d)
_, err := stack.Check(ctx, db, tc.tx)
if tc.err != nil {
require.EqualError(t, err, tc.err.Error())
} else {
require.NoError(t, err)
assert.Equal(t, tc.perms, hn.Perms)
if !tc.wantErr.Is(err) {
t.Fatalf("unexpected error: %+v", err)
}

_, err = stack.Deliver(ctx, db, tc.tx)
if tc.err != nil {
require.EqualError(t, err, tc.err.Error())
} else {
require.NoError(t, err)
assert.Equal(t, tc.perms, hn.Perms)
if !tc.wantErr.Is(err) {
t.Fatalf("unexpected error: %+v", err)
}
})
}
Expand Down Expand Up @@ -166,7 +159,7 @@ type ContractTx struct {
MultisigID [][]byte
}

var _ MultiSigTx = ContractTx{}
var _ multiSigTx = ContractTx{}
var _ weave.Tx = ContractTx{}

func (p ContractTx) GetMultisig() [][]byte {
Expand Down
36 changes: 16 additions & 20 deletions x/multisig/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,40 +125,36 @@ func (h UpdateContractMsgHandler) Deliver(ctx weave.Context, db weave.KVStore, t
return res, nil
}

// validate does all common pre-processing between Check and Deliver
func (h UpdateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx weave.Tx) (*UpdateContractMsg, error) {
msg, err := tx.GetMsg()
txmsg, err := tx.GetMsg()
if err != nil {
return nil, err
return nil, errors.Wrap(err, "cannot get transaction message")
}

updateContractMsg, ok := msg.(*UpdateContractMsg)
msg, ok := txmsg.(*UpdateContractMsg)
if !ok {
return nil, errors.Wrapf(errors.ErrInvalidType, "%T", msg)
}

err = updateContractMsg.Validate()
if err != nil {
if err := msg.Validate(); err != nil {
return nil, err
}

obj, err := h.bucket.Get(db, updateContractMsg.ContractID)
// Using current version of the contract, ensure that enoguht
// participants with enought power/weight signed this transaction in
// order to run functionality that requires admin rights.
contract, err := h.bucket.GetContract(db, msg.ContractID)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "bucket lookup")
}
if obj == nil || (obj != nil && obj.Value() == nil) {
return nil, errors.ErrNotFound
}
contract := obj.Value().(*Contract)

var sigs []weave.Address
var power Weight
for _, p := range contract.Participants {
sigs = append(sigs, p.Signature)
if h.auth.HasAddress(ctx, p.Signature) {
power += p.Power
}
}
authenticated := x.HasNAddresses(ctx, h.auth, sigs, int(contract.AdminThreshold))
if !authenticated {
return nil, errors.ErrUnauthorized
if power < contract.AdminThreshold {
return msg, errors.Wrapf(errors.ErrUnauthorized, "%d power is not enough", power)
}

return updateContractMsg, nil
return msg, nil
}
Loading

0 comments on commit 53b295a

Please sign in to comment.