Fix ABI Address packing issue and error swallowing

- Burrow deploy was swallowing errors in multiple playbook runs
- 0.29.8 ABI fix accidentally removed support for hex string addresses -
corrected!
- Make console logging look more reasonable
- Fix ability to call fallback function

Signed-off-by: Silas Davis <[email protected]>

Author: Silas Davis

cmd/burrow/commands/deploy.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hyperledger/burrow/deploy/def"
 	"github.com/hyperledger/burrow/deploy/proposals"
 	"github.com/hyperledger/burrow/logging"
+	"github.com/hyperledger/burrow/logging/loggers"
 	cli "github.com/jawher/mow.cli"
 )
 
@@ -34,9 +35,6 @@ func Deploy(output Output) func(cmd *cli.Cmd) {
 		defaultOutputOpt := cmd.StringOpt("o output", def.DefaultOutputFile,
 			"filename for playbook output file. by default, this name will reflect the playbook passed")
 
-		playbooksOpt := cmd.StringsArg("FILE", []string{},
-			"path to playbook file which deploy should run. if also using the --dir flag, give the relative path to playbooks file, which should be in the same directory")
-
 		defaultSetsOpt := cmd.StringsOpt("e set", []string{},
 			"default sets to use; operates the same way as the [set] jobs, only before the jobs file is ran (and after default address")
 
@@ -75,6 +73,9 @@ func Deploy(output Output) func(cmd *cli.Cmd) {
 
 		proposalList := cmd.StringOpt("list-proposals state", "", "List proposals, either all, executed, expired, or current")
 
+		playbooksArg := cmd.StringsArg("FILE", []string{},
+			"path to playbook file which deploy should run. if also using the --dir flag, give the relative path to playbooks file, which should be in the same directory")
+
 		cmd.Spec = "[--chain=<host:port>] [--keys=<host:port>] [--mempool-signing] [--dir=<root directory>] " +
 			"[--output=<output file>] [--wasm] [--set=<KEY=VALUE>]... [--bin-path=<path>] [--gas=<gas>] " +
 			"[--jobs=<concurrent playbooks>] [--address=<address>] [--fee=<fee>] [--amount=<amount>] [--local-abi] " +
@@ -115,7 +116,10 @@ func Deploy(output Output) func(cmd *cli.Cmd) {
 			args.ProposeVerify = *proposalVerify
 			args.ProposeVote = *proposalVote
 			args.ProposeCreate = *proposalCreate
-			stderrLogger := log.NewLogfmtLogger(os.Stderr)
+			stderrLogger, err := loggers.NewStreamLogger(os.Stderr, loggers.TerminalFormat)
+			if err != nil {
+				output.Fatalf("Could not make logger: %v", err)
+			}
 			logger := logging.NewLogger(stderrLogger)
 			handleTerm()
 
@@ -133,10 +137,10 @@ func Deploy(output Output) func(cmd *cli.Cmd) {
 					output.Fatalf(err.Error())
 				}
 			} else {
-				if len(*playbooksOpt) == 0 {
+				if len(*playbooksArg) == 0 {
 					output.Fatalf("incorrect usage: missing deployment yaml file(s)")
 				}
-				failures, err := pkgs.RunPlaybooks(args, *playbooksOpt, logger)
+				failures, err := pkgs.RunPlaybooks(args, *playbooksArg, logger)
 				if err != nil {
 					fmt.Fprintln(os.Stderr, err)
 					os.Exit(1)

deploy/def/jobs.go

@@ -1,6 +1,7 @@
 package def
 
 import (
+	"fmt"
 	"regexp"
 
 	validation "github.com/go-ozzo/ozzo-validation"
@@ -405,6 +406,11 @@ type Call struct {
 	Variables []*abi.Variable
 }
 
+// TODO: maybe do for others...
+func (job *Call) String() string {
+	return fmt.Sprintf("%#v", job)
+}
+
 func (job *Call) Validate() error {
 	return validation.ValidateStruct(job,
 		validation.Field(&job.Destination, validation.Required),

deploy/jobs/jobs_contracts.go

@@ -7,15 +7,14 @@ import (
 	"strings"
 
 	"github.com/hyperledger/burrow/acm/acmstate"
-	"github.com/hyperledger/burrow/execution/errors"
-	"github.com/hyperledger/burrow/execution/exec"
-	"github.com/hyperledger/burrow/logging"
-
 	"github.com/hyperledger/burrow/crypto"
 	compilers "github.com/hyperledger/burrow/deploy/compile"
 	"github.com/hyperledger/burrow/deploy/def"
 	"github.com/hyperledger/burrow/deploy/util"
+	"github.com/hyperledger/burrow/execution/errors"
 	"github.com/hyperledger/burrow/execution/evm/abi"
+	"github.com/hyperledger/burrow/execution/exec"
+	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/txs/payload"
 	hex "github.com/tmthrgd/go-hex"
 )
@@ -431,6 +430,7 @@ func FormulateCallJob(call *def.Call, do *def.DeployArgs, deployScript *def.Play
 	if err != nil {
 		return nil, err
 	}
+
 	// Use default
 	call.Source = FirstOf(call.Source, deployScript.Account)
 	call.Amount = FirstOf(call.Amount, do.DefaultAmount)

deploy/run_deploy.go

@@ -112,16 +112,18 @@ func RunPlaybooks(args *def.DeployArgs, playbooks []string, logger *logging.Logg
 	successes := 0
 
 	for range playbooks {
+		// Receive results as they come
 		jobResult := <-resultQ
 		results[jobResult.jobNo] = &jobResult
+		// Print them in order
 		for results[printed] != nil {
 			res := results[printed]
 			os.Stderr.Write(res.log.Bytes())
 			if res.err != nil {
-				fmt.Fprintf(os.Stderr, "ERROR: %v", res.err)
+				fmt.Fprintf(os.Stderr, "Error in RunPlaybooks: %v\n", res.err)
 			}
 			res.log.Truncate(0)
-			if jobResult.err != nil {
+			if res.err != nil {
 				failures++
 			} else {
 				successes++
@@ -135,7 +137,7 @@ func RunPlaybooks(args *def.DeployArgs, playbooks []string, logger *logging.Logg
 	close(resultQ)
 
 	if successes > 0 {
-		logger.InfoMsg("JOBS THAT SUCCEEEDED", "count", successes)
+		logger.InfoMsg("JOBS THAT SUCCEEDED", "count", successes)
 		for i, playbook := range playbooks {
 			res := results[i]
 			if res.err != nil {

deploy/util/variables.go

@@ -195,12 +195,10 @@ func replaceBlockVariable(toReplace string, client *def.Client, logger *logging.
 func PreProcessInputData(function string, data interface{}, do *def.DeployArgs, script *def.Playbook, client *def.Client, constructor bool, logger *logging.Logger) (string, []interface{}, error) {
 	var callDataArray []interface{}
 	var callArray []string
-	if function == "" && !constructor {
-		if reflect.TypeOf(data).Kind() == reflect.Slice {
-			return "", []interface{}{""}, fmt.Errorf("Incorrect formatting of deploy.yaml. Please update it to include a function field.")
-		}
-		function = strings.Split(data.(string), " ")[0]
-		callArray = strings.Split(data.(string), " ")[1:]
+	if _, ok := data.(string); function == "" && !constructor && ok {
+		callArray = strings.Split(data.(string), " ")
+		function = callArray[0]
+		callArray = callArray[1:]
 		for _, val := range callArray {
 			output, _ := PreProcess(val, do, script, client, logger)
 			callDataArray = append(callDataArray, output)

docker-compose.yml

@@ -2,6 +2,8 @@ version: "3"
 services:
   db:
     image: postgres:11-alpine
+    environment:
+      - POSTGRES_HOST_AUTH_METHOD=trust
     ports:
       - 5432
     environment:

execution/evm/abi/abi_test.go

@@ -26,6 +26,13 @@ func TestPacker(t *testing.T) {
 		name           string
 		expectedOutput []byte
 	}{
+		// Test string address
+		{
+			`[{"constant":false,"inputs":[{"internalType":"address payable","name":"friend","type":"address"}],"name":"sendToAFriend","outputs":[],"payable":true,"stateMutability":"payable","type":"function"}]`,
+			[]interface{}{"C42DEED84BDF2CA695F2E91F4E6395D191CF35FC"},
+			"sendToAFriend",
+			pad([]byte{196, 45, 238, 216, 75, 223, 44, 166, 149, 242, 233, 31, 78, 99, 149, 209, 145, 207, 53, 252}, 32, true),
+		},
 		// From: https://github.com/hyperledger/burrow/issues/1326
 		{
 			`[{"constant":false,"inputs":[{"internalType":"address payable","name":"friend","type":"address"}],"name":"sendToAFriend","outputs":[],"payable":true,"stateMutability":"payable","type":"function"}]`,

execution/evm/abi/primitives.go

@@ -520,10 +520,16 @@ func (e EVMAddress) pack(v interface{}) ([]byte, error) {
 		bs = a[:]
 	case *crypto.Address:
 		bs = (*a)[:]
+	case string:
+		address, err := crypto.AddressFromHexString(a)
+		if err != nil {
+			return nil, fmt.Errorf("could not convert '%s' to address: %v", a, err)
+		}
+		bs = address[:]
 	case []byte:
 		address, err := crypto.AddressFromBytes(a)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("could not convert byte 0x%X to address: %v", a, err)
 		}
 		bs = address[:]
 	default:

execution/evm/abi/spec.go

@@ -10,6 +10,9 @@ import (
 	"github.com/hyperledger/burrow/crypto"
 )
 
+// Token to use in deploy yaml in order to indicate call to the fallback function.
+const FallbackFunctionName = "()"
+
 // Spec is the ABI for contract decoded.
 type Spec struct {
 	Constructor  *FunctionSpec
@@ -69,6 +72,8 @@ func ReadSpec(specBytes []byte) (*Spec, error) {
 		case "fallback":
 			abiSpec.Fallback.Inputs = make([]Argument, 0)
 			abiSpec.Fallback.Outputs = make([]Argument, 0)
+			abiSpec.Fallback.SetConstant()
+			abiSpec.Functions[FallbackFunctionName] = abiSpec.Fallback
 		case "event":
 			ev := new(EventSpec)
 			err = ev.unmarshalSpec(&s)
@@ -136,13 +141,13 @@ func (spec *Spec) Pack(fname string, args ...interface{}) ([]byte, *FunctionSpec
 		if _, ok := spec.Functions[fname]; ok {
 			funcSpec = spec.Functions[fname]
 		} else {
-			return nil, nil, fmt.Errorf("Unknown function %s", fname)
+			return nil, nil, fmt.Errorf("unknown function in Pack: %s", fname)
 		}
 	} else {
 		if spec.Constructor.Inputs != nil {
 			funcSpec = spec.Constructor
 		} else {
-			return nil, nil, fmt.Errorf("Contract does not have a constructor")
+			return nil, nil, fmt.Errorf("contract does not have a constructor")
 		}
 	}
 
@@ -179,7 +184,7 @@ func (spec *Spec) Unpack(data []byte, fname string, args ...interface{}) error {
 	argSpec = funcSpec.Outputs
 
 	if argSpec == nil {
-		return fmt.Errorf("Unknown function %s", fname)
+		return fmt.Errorf("unknown function in Unpack: %s", fname)
 	}
 
 	return unpack(argSpec, data, func(i int) interface{} {
@@ -199,7 +204,7 @@ func (spec *Spec) UnpackWithID(data []byte, args ...interface{}) error {
 	}
 
 	if argSpec == nil {
-		return fmt.Errorf("Unknown function %x", id)
+		return fmt.Errorf("unknown function in UnpackWithID: %x", id)
 	}
 
 	return unpack(argSpec, data[4:], func(i int) interface{} {

logging/loggers/stream_logger.go

@@ -15,56 +15,70 @@ type Syncable interface {
 }
 
 func NewStreamLogger(writer io.Writer, format string) (log.Logger, error) {
-	var logger log.Logger
-	var err error
 	switch format {
 	case "":
 		return NewStreamLogger(writer, DefaultFormat)
 	case JSONFormat:
-		logger = log.NewJSONLogger(writer)
+		return NewJSONLogger(writer), nil
 	case LogfmtFormat:
-		logger = log.NewLogfmtLogger(writer)
+		return NewLogfmtLogger(writer), nil
 	case TerminalFormat:
-		logger = term.NewLogger(writer, log.NewLogfmtLogger, func(keyvals ...interface{}) term.FgBgColor {
-			switch structure.Value(keyvals, structure.ChannelKey) {
-			case structure.TraceChannelName:
-				return term.FgBgColor{Fg: term.DarkGreen}
-			default:
-				return term.FgBgColor{Fg: term.Yellow}
-			}
-		})
+		return NewTerminalLogger(writer), nil
 	default:
-		logger, err = NewTemplateLogger(writer, format, []byte{})
-		if err != nil {
-			return nil, fmt.Errorf("did not recognise format '%s' as named format and could not parse as "+
-				"template: %v", format, err)
-		}
+		return NewTemplateLogger(writer, format, []byte{})
 	}
-	return log.LoggerFunc(func(keyvals ...interface{}) error {
-		switch structure.Signal(keyvals) {
-		case structure.SyncSignal:
-			if s, ok := writer.(Syncable); ok {
-				return s.Sync()
-			}
-			// Don't log signals
-			return nil
+}
+
+func NewJSONLogger(writer io.Writer) log.Logger {
+	return interceptSync(writer, log.NewJSONLogger(writer))
+}
+
+func NewLogfmtLogger(writer io.Writer) log.Logger {
+	return interceptSync(writer, log.NewLogfmtLogger(writer))
+}
+
+func NewTerminalLogger(writer io.Writer) log.Logger {
+	logger := term.NewLogger(writer, log.NewLogfmtLogger, func(keyvals ...interface{}) term.FgBgColor {
+		kvm := structure.KeyValuesMap(keyvals)
+		delete(kvm, structure.TraceChannelName)
+		delete(kvm, structure.MessageKey)
+		delete(kvm, structure.TimeKey)
+		switch structure.Value(keyvals, structure.ChannelKey) {
+		case structure.TraceChannelName:
+			return term.FgBgColor{Fg: term.DarkGreen}
 		default:
-			return logger.Log(keyvals...)
+			return term.FgBgColor{Fg: term.Yellow}
 		}
-	}), nil
+	})
+	return interceptSync(writer, logger)
 }
 
 func NewTemplateLogger(writer io.Writer, textTemplate string, recordSeparator []byte) (log.Logger, error) {
 	tmpl, err := template.New("template-logger").Parse(textTemplate)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not parse '%s' as a text template: %v", textTemplate, err)
 	}
-	return log.LoggerFunc(func(keyvals ...interface{}) error {
+	logger := log.LoggerFunc(func(keyvals ...interface{}) error {
 		err := tmpl.Execute(writer, structure.KeyValuesMap(keyvals))
 		if err == nil {
 			_, err = writer.Write(recordSeparator)
 		}
 		return err
-	}), nil
+	})
+	return interceptSync(writer, logger), nil
+}
 
+func interceptSync(writer io.Writer, logger log.Logger) log.Logger {
+	return log.LoggerFunc(func(keyvals ...interface{}) error {
+		switch structure.Signal(keyvals) {
+		case structure.SyncSignal:
+			if s, ok := writer.(Syncable); ok {
+				return s.Sync()
+			}
+			// Don't log signals
+			return nil
+		default:
+			return logger.Log(keyvals...)
+		}
+	})
 }

tests/jobs_fixtures/app37-fallback_function/deploy.yaml

@@ -11,10 +11,10 @@ jobs:
 - name: getX
   query-contract:
       destination: $deployC
-      function: x 
+      function: x
 
 - name: assertLevelUp
   assert:
       key: $getX
       relation: eq
-      val: 1
+      val: 1

tests/test_runner.sh

@@ -25,8 +25,6 @@ export boot=${boot:-true}
 export debug=${debug:-false}
 export clean=${clean:-true}
 
-export failures="not supplied by test"
-
 export test_exit=0
 
 if [[ "$debug" = true ]]; then
@@ -114,11 +112,6 @@ test_teardown(){
     echo "Tests complete! Tests are Green. :)"
   else
     echo "Tests complete. Tests are Red. :("
-    echo "Failures in:"
-    for failure in "${failures[@]}"
-    do
-      echo "$failure"
-    done
    fi
   exit ${test_exit}
 }