Merge pull request #8855 from Agoric/gibson-gh-8703-followup

docs(golang): Improve the description of PopulateAction

golang/cosmos/vm/action.go

@@ -58,21 +58,23 @@ func SetActionHeaderFromContext(ctx sdk.Context, actionType string, ah *ActionHe
 	}
 }
 
-// PopulateAction interprets `default:"..."` tags and specially handles an
-// embedded ActionHeader struct.
+// PopulateAction returns a clone of action in which empty/zero-valued fields
+// in its embedded ActionHeader have been populated using the corresponding
+// `actionType:"..."` tag and the provided ctx, and its own empty/zero-valued
+// fields have been populated as specified by their `default:"..."` tags.
 func PopulateAction(ctx sdk.Context, action Action) Action {
-	oldRv := reflect.Indirect(reflect.ValueOf(action))
-	if oldRv.Kind() != reflect.Struct {
+	oldActionDesc := reflect.Indirect(reflect.ValueOf(action))
+	if oldActionDesc.Kind() != reflect.Struct {
 		return action
 	}
 
 	// Shallow copy to a new value.
-	rp := reflect.New(oldRv.Type())
-	rv := reflect.Indirect(rp)
-	for i := 0; i < rv.NumField(); i++ {
-		oldField := oldRv.Field(i)
-		field := rv.Field(i)
-		fieldType := rv.Type().Field(i)
+	newActionDescPtr := reflect.New(oldActionDesc.Type())
+	newActionDesc := reflect.Indirect(newActionDescPtr)
+	for i := 0; i < newActionDesc.NumField(); i++ {
+		oldField := oldActionDesc.Field(i)
+		field := newActionDesc.Field(i)
+		fieldType := newActionDesc.Type().Field(i)
 		if !field.CanSet() {
 			continue
 		}
@@ -81,34 +83,35 @@ func PopulateAction(ctx sdk.Context, action Action) Action {
 		field.Set(oldField)
 
 		// Populate any ActionHeader struct.
-		var ahp *ActionHeader
+		var headerPtr *ActionHeader
 		if fieldType.Type == actionHeaderType {
-			ahp = field.Addr().Interface().(*ActionHeader)
+			headerPtr = field.Addr().Interface().(*ActionHeader)
 		} else if fieldType.Type == reflect.PtrTo(actionHeaderType) {
 			if field.IsNil() {
-				ahp = &ActionHeader{}
+				headerPtr = &ActionHeader{}
 			} else {
-				ahp = field.Interface().(*ActionHeader)
+				headerPtr = field.Interface().(*ActionHeader)
 			}
 		}
-		if ahp != nil {
+		if headerPtr != nil {
 			actionTypeTag, _ := fieldType.Tag.Lookup("actionType")
-			ah := *ahp
-			SetActionHeaderFromContext(ctx, actionTypeTag, &ah)
+			newHeader := *headerPtr
+			SetActionHeaderFromContext(ctx, actionTypeTag, &newHeader)
 			if field.Kind() == reflect.Ptr {
-				field.Set(reflect.ValueOf(&ah))
+				field.Set(reflect.ValueOf(&newHeader))
 			} else {
-				field.Set(reflect.ValueOf(ah))
+				field.Set(reflect.ValueOf(newHeader))
 			}
 			continue
 		}
 
-		// Still zero value, try default struct field tag.
+		// Skip any field that is already populated or lacks a "default" tag.
 		defaultTag, _ := fieldType.Tag.Lookup("default")
 		if !field.IsZero() || len(defaultTag) == 0 {
 			continue
 		}
 
+		// Populate the field from its "default" tag.
 		switch field.Kind() {
 		case reflect.String:
 			field.SetString(defaultTag)
@@ -126,5 +129,5 @@ func PopulateAction(ctx sdk.Context, action Action) Action {
 			}
 		}
 	}
-	return rv.Interface().(Action)
+	return newActionDesc.Interface().(Action)
 }

golang/cosmos/vm/action_test.go

@@ -106,21 +106,21 @@ func TestActionContext(t *testing.T) {
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			jstr := func(in interface{}) string {
+			toJson := func(in interface{}) string {
 				bz, err := json.Marshal(in)
 				if err != nil {
 					t.Fatal(err)
 				}
 				return string(bz)
 			}
-			jsonIn := jstr(tc.in)
+			jsonIn := toJson(tc.in)
 			out := vm.PopulateAction(tc.ctx, tc.in)
-			jsonIn2 := jstr(tc.in)
+			jsonIn2 := toJson(tc.in)
 			if jsonIn != jsonIn2 {
 				t.Errorf("unexpected mutated input: %s to %s", jsonIn, jsonIn2)
 			}
-			jsonOut := jstr(out)
-			jsonExpectedOut := jstr(tc.expectedOut)
+			jsonOut := toJson(out)
+			jsonExpectedOut := toJson(tc.expectedOut)
 			if jsonOut != jsonExpectedOut {
 				t.Errorf("expected %s, got %s", jsonExpectedOut, jsonOut)
 			}