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

refactor: Improve txRaw decoder code #9769

Merged
merged 10 commits into from
Jul 26, 2021
41 changes: 23 additions & 18 deletions x/auth/tx/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
func DefaultTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder {
return func(txBytes []byte) (sdk.Tx, error) {
// Make sure txBytes follow ADR-027.
err := rejectNonADR027(txBytes)
err := rejectNonADR027TxRaw(txBytes)
if err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrTxDecode, err.Error())
}
Expand Down Expand Up @@ -90,13 +90,14 @@ func DefaultJSONTxDecoder(cdc codec.ProtoCodecMarshaler) sdk.TxDecoder {
}
}

// rejectNonADR027 rejects txBytes that do not follow ADR-027. This function
// rejectNonADR027TxRaw rejects txBytes that do not follow ADR-027. This is NOT
// a generic ADR-027 checker, it only applies decoding TxRaw. Specifically, it
// only checks that:
// - field numbers are in ascending order (1, 2, and potentially multiple 3s),
// - and varints as as short as possible.
// All other ADR-027 edge cases (e.g. TxRaw fields having default values) will
// not happen with TxRaw.
func rejectNonADR027(txBytes []byte) error {
// - and varints are as short as possible.
// All other ADR-027 edge cases (e.g. default values) are not applicable with
// TxRaw.
func rejectNonADR027TxRaw(txBytes []byte) error {
// Make sure all fields are ordered in ascending order with this variable.
prevTagNum := protowire.Number(0)

Expand All @@ -105,21 +106,25 @@ func rejectNonADR027(txBytes []byte) error {
if m < 0 {
return fmt.Errorf("invalid length; %w", protowire.ParseError(m))
}
// TxRaw only has bytes fields.
if wireType != protowire.BytesType {
return fmt.Errorf("expected %d wire type, got %d", protowire.VarintType, wireType)
return fmt.Errorf("expected %d wire type, got %d", protowire.BytesType, wireType)
}
// Make sure fields are ordered in ascending order.
if tagNum < prevTagNum {
return fmt.Errorf("txRaw must follow ADR-027, got tagNum %d after tagNum %d", tagNum, prevTagNum)
}
prevTagNum = tagNum

// All 3 fields of TxRaw have wireType == 2, so their next component
// is a varint.
// We make sure that the varint is as short as possible.
// is a varint, so we can safely call ConsumeVarint here.
// Byte structure: <varint of bytes length><bytes sequence>
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
// Inner fields are verified in `DefaultTxDecoder`
lengthPrefix, m := protowire.ConsumeVarint(txBytes[m:])
if m < 0 {
return fmt.Errorf("invalid length; %w", protowire.ParseError(m))
}
// We make sure that this varint is as short as possible.
n := varintMinLength(lengthPrefix)
if n != m {
return fmt.Errorf("length prefix varint for tagNum %d is not as short as possible, read %d, only need %d", tagNum, m, n)
Expand All @@ -141,23 +146,23 @@ func rejectNonADR027(txBytes []byte) error {
func varintMinLength(n uint64) int {
switch {
// Note: 1<<N == 2**N.
case n < 1<<7:
case n < 1<<(7):
return 1
case n < 1<<14:
case n < 1<<(7*2):
return 2
case n < 1<<21:
case n < 1<<(7*3):
return 3
case n < 1<<28:
case n < 1<<(7*4):
return 4
case n < 1<<35:
case n < 1<<(7*5):
return 5
case n < 1<<42:
case n < 1<<(7*6):
return 6
case n < 1<<49:
case n < 1<<(7*7):
return 7
case n < 1<<56:
case n < 1<<(7*8):
return 8
case n < 1<<63:
case n < 1<<(7*9):
return 9
default:
return 10
Expand Down