Skip to content

Commit

Permalink
common/cbor: Add debug.strict_cbor
Browse files Browse the repository at this point in the history
We might as well add round-trip sanity checking, at least when we are
testing things.  I'm not sure if this should be enabled all the time,
and it is written under the assumption that it is not, as round-trip
failures will crash the node if the flag is enabled.
  • Loading branch information
Yawning committed Oct 14, 2019
1 parent 0066a4d commit bb1f4f5
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 1 deletion.
41 changes: 40 additions & 1 deletion go/common/cbor/cbor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,21 @@
// to always have the same serialization.
package cbor

import "github.com/fxamacker/cbor"
import (
"bytes"
"encoding/hex"
"fmt"

"github.com/fxamacker/cbor"
flag "github.com/spf13/pflag"
"github.com/spf13/viper"
)

// CfgDebugStrictCBOR enables CBOR round-trip enforcement.
const CfgDebugStrictCBOR = "debug.strict_cbor"

// Flags has the flags used by the CBOR wrapper.
var Flags = flag.NewFlagSet("", flag.ContinueOnError)

// FixSliceForSerde will convert `nil` to `[]byte` to work around serde
// brain damage.
Expand Down Expand Up @@ -46,6 +60,25 @@ func Unmarshal(data []byte, dst interface{}) error {
return nil
}

err := cbor.Unmarshal(data, dst)
if err != nil {
return err
}

// If we are running with the strict CBOR debug option, ensure that
// the structure round-trips.
if viper.GetBool(CfgDebugStrictCBOR) {
reencoded := Marshal(dst)
if !bytes.Equal(data, reencoded) {
msg := fmt.Sprintf(
"common/cbor: encoded %T does not round-trip (expected: %s, actual: %s)",
hex.EncodeToString(data),
hex.EncodeToString(reencoded),
)
panic(msg)
}
}

return cbor.Unmarshal(data, dst)
}

Expand All @@ -56,3 +89,9 @@ func MustUnmarshal(data []byte, dst interface{}) {
panic(err)
}
}

func init() {
Flags.Bool(CfgDebugStrictCBOR, false, "(DEBUG) Enforce that CBOR blobs roundtrip")

_ = viper.BindPFlags(Flags)
}
4 changes: 4 additions & 0 deletions go/oasis-node/cmd/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spf13/viper"

"github.com/oasislabs/oasis-core/go/common"
"github.com/oasislabs/oasis-core/go/common/cbor"
"github.com/oasislabs/oasis-core/go/common/crypto/signature"
fileSigner "github.com/oasislabs/oasis-core/go/common/crypto/signature/signers/file"
"github.com/oasislabs/oasis-core/go/common/entity"
Expand Down Expand Up @@ -95,6 +96,9 @@ func init() {
RootFlags.Bool(CfgDebugAllowTestKeys, false, "Allow test keys (UNSAFE)")
_ = viper.BindPFlags(RootFlags)
RootFlags.AddFlagSet(loggingFlags)

// Common debugging flags.
RootFlags.AddFlagSet(cbor.Flags)
}

// InitConfig initializes the command configuration.
Expand Down
5 changes: 5 additions & 0 deletions go/oasis-test-runner/oasis/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ type argBuilder struct {
vec []string
}

func (args *argBuilder) debugStrictCBOR() *argBuilder {
args.vec = append(args.vec, "--"+cbor.CfgDebugStrictCBOR)
return args
}

func (args *argBuilder) debugAllowTestKeys() *argBuilder {
args.vec = append(args.vec, "--"+cmdCommon.CfgDebugAllowTestKeys)
return args
Expand Down
1 change: 1 addition & 0 deletions go/oasis-test-runner/oasis/oasis.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ func (net *Network) startOasisNode(
registryDebugAllowUnroutableAddresses().
tendermintDebugAddrBookLenient()
}
extraArgs = extraArgs.debugStrictCBOR()
args := append([]string{}, subCmd...)
args = append(args, baseArgs...)
args = append(args, extraArgs.vec...)
Expand Down

0 comments on commit bb1f4f5

Please sign in to comment.