Skip to content

Commit

Permalink
Fix direct_aux sign mode handler
Browse files Browse the repository at this point in the history
  • Loading branch information
amaury1093 committed Oct 26, 2021
1 parent a424a34 commit c5bfc3b
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 20 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#10326](https://github.com/cosmos/cosmos-sdk/pull/10326) `x/authz` add query all grants by granter query.
* [\#10024](https://github.com/cosmos/cosmos-sdk/pull/10024) `store/cachekv` performance improvement by reduced growth factor for iterator ranging by using binary searches to find dirty items when unsorted key count >= 1024
* [\#10348](https://github.com/cosmos/cosmos-sdk/pull/10348) Add `fee.{payer,granter}` and `tip` fields to StdSignDoc for signing tipped transactions.
* [\#10208](https://github.com/cosmos/cosmos-sdk/pull/10208) Add `SignModeTxMiddleware` for checking sign modes and `TipsTxMiddleware` for transferring tips.
* [\#10208](https://github.com/cosmos/cosmos-sdk/pull/10208) Add `TipsTxMiddleware` for transferring tips.

### API Breaking Changes

Expand Down
8 changes: 4 additions & 4 deletions types/tx/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ func (t *Tx) GetSigners() []sdk.AccAddress {
}

// ensure any specified fee payer is included in the required signers (at the end)
feePayer := t.AuthInfo.Fee.Payer
if feePayer != "" && !seen[feePayer] {
payerAddr, err := sdk.AccAddressFromBech32(feePayer)
fee := t.AuthInfo.Fee
if fee != nil && fee.Payer != "" && !seen[fee.Payer] {
payerAddr, err := sdk.AccAddressFromBech32(fee.Payer)
if err != nil {
panic(err)
}
signers = append(signers, payerAddr)
seen[feePayer] = true
seen[fee.Payer] = true
}

return signers
Expand Down
12 changes: 5 additions & 7 deletions x/auth/tx/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@ type ExtensionOptionsTxBuilder interface {
func newBuilder() *wrapper {
return &wrapper{
tx: &tx.Tx{
Body: &tx.TxBody{},
AuthInfo: &tx.AuthInfo{
Fee: &tx.Fee{},
},
Body: &tx.TxBody{},
AuthInfo: &tx.AuthInfo{},
},
}
}
Expand Down Expand Up @@ -133,9 +131,9 @@ func (w *wrapper) GetFee() sdk.Coins {
}

func (w *wrapper) FeePayer() sdk.AccAddress {
feePayer := w.tx.AuthInfo.Fee.Payer
if feePayer != "" {
payerAddr, err := sdk.AccAddressFromBech32(feePayer)
fee := w.tx.AuthInfo.Fee
if fee != nil && fee.Payer != "" {
payerAddr, err := sdk.AccAddressFromBech32(fee.Payer)
if err != nil {
panic(err)
}
Expand Down
36 changes: 28 additions & 8 deletions x/auth/tx/direct_aux.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,38 @@ func (signModeDirectAuxHandler) GetSignBytes(
return nil, sdkerrors.ErrInvalidRequest.Wrapf("got empty pubkey for signer #%d in %s handler", data.SignerIndex, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)
}

// Fee payer cannot use SIGN_MODE_DIRECT_AUX, because SIGN_MODE_DIRECT_AUX
// does not sign over fees, which would create malleability issues.
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return nil, sdkerrors.ErrInvalidType.Wrapf("expected %T, got %T", sdk.FeeTx(nil), tx)
}
addr := data.Address
if addr == "" {
return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "got empty address in %s handler", signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)
}
if feeTx.FeePayer().String() == data.Address {
return nil, sdkerrors.ErrUnauthorized.Wrapf("fee payer %s cannot sign with %s", feeTx.FeePayer(), signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)

feePayer := protoTx.FeePayer().String()

// Fee payer cannot use SIGN_MODE_DIRECT_AUX, because SIGN_MODE_DIRECT_AUX
// does not sign over fees, which would create malleability issues.
if feePayer == data.Address {
tip := protoTx.tx.GetAuthInfo().GetTip()
var tipper string
if tip != nil {
tipper = tip.Tipper
}

// In general, the transactions with tips require that the fee payer and
// tipper are two different persons.
//
// However, recall that `protoTx.FeePayer()` is defined as
// `tx.AuthInfo.Fee.Payer` if not nil, or defaults to `tx.GetSigners[0]`.
// When fee payer is `tx.GetSigners[0]` (i.e. the tx.AuthInfo.Fee.Payer
// field is not set), then the tipper and the fee payer
// are the same person. Concretely, this happens when the tipper signs
// their tx before relaying it to the fee payer.
if tipper == feePayer {
if protoTx.tx.GetAuthInfo().GetFee() != nil {
return nil, sdkerrors.ErrUnauthorized.Wrapf("tipper %s cannot be fee payer", tipper)
}
} else {
return nil, sdkerrors.ErrUnauthorized.Wrapf("fee payer %s cannot sign with %s", feePayer, signingtypes.SignMode_SIGN_MODE_DIRECT_AUX)
}
}

signDocDirectAux := types.SignDocDirectAux{
Expand Down

0 comments on commit c5bfc3b

Please sign in to comment.