From bb1f4f55f8d3fe3d413daca2ad0ddba03b400c3b Mon Sep 17 00:00:00 2001 From: Yawning Angel Date: Mon, 14 Oct 2019 09:40:07 +0000 Subject: [PATCH] common/cbor: Add `debug.strict_cbor` 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. --- go/common/cbor/cbor.go | 41 ++++++++++++++++++++++++++++- go/oasis-node/cmd/common/common.go | 4 +++ go/oasis-test-runner/oasis/args.go | 5 ++++ go/oasis-test-runner/oasis/oasis.go | 1 + 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/go/common/cbor/cbor.go b/go/common/cbor/cbor.go index bed470a576e..94bddc12ad5 100644 --- a/go/common/cbor/cbor.go +++ b/go/common/cbor/cbor.go @@ -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. @@ -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) } @@ -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) +} diff --git a/go/oasis-node/cmd/common/common.go b/go/oasis-node/cmd/common/common.go index f2066d24930..fd0b88d22b8 100644 --- a/go/oasis-node/cmd/common/common.go +++ b/go/oasis-node/cmd/common/common.go @@ -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" @@ -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. diff --git a/go/oasis-test-runner/oasis/args.go b/go/oasis-test-runner/oasis/args.go index e8945539a47..4dd404ec562 100644 --- a/go/oasis-test-runner/oasis/args.go +++ b/go/oasis-test-runner/oasis/args.go @@ -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 diff --git a/go/oasis-test-runner/oasis/oasis.go b/go/oasis-test-runner/oasis/oasis.go index ad805e1eb6f..b05df39f7d2 100644 --- a/go/oasis-test-runner/oasis/oasis.go +++ b/go/oasis-test-runner/oasis/oasis.go @@ -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...)