accounts/abi: Fix method overwritten by same name methods. (#17099)

* accounts/abi: Fix method overwritten by same name methods.

* accounts/abi: Fix method overwritten by same name methods.

* accounts/abi: avoid possible name conflict

Co-authored-by: Guillaume Ballet <gballet@gmail.com>

3 files changed, 58 insertions, 33 deletions

diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go
index c3d809e5a..97a3a98bd 100644
--- a/accounts/abi/abi.go
+++ b/accounts/abi/abi.go
@@ -136,15 +136,27 @@ func (abi *ABI) UnmarshalJSON(data []byte) error {
 			}
 		// empty defaults to function according to the abi spec
 		case "function", "":
-			abi.Methods[field.Name] = Method{
-				Name:    field.Name,
+			name := field.Name
+			_, ok := abi.Methods[name]
+			for idx := 0; ok; idx++ {
+				name = fmt.Sprintf("%s%d", field.Name, idx)
+				_, ok = abi.Methods[name]
+			}
+			abi.Methods[name] = Method{
+				Name:    name,
 				Const:   field.Constant,
 				Inputs:  field.Inputs,
 				Outputs: field.Outputs,
 			}
 		case "event":
-			abi.Events[field.Name] = Event{
-				Name:      field.Name,
+			name := field.Name
+			_, ok := abi.Events[name]
+			for idx := 0; ok; idx++ {
+				name = fmt.Sprintf("%s%d", field.Name, idx)
+				_, ok = abi.Events[name]
+			}
+			abi.Events[name] = Event{
+				Name:      name,
 				Anonymous: field.Anonymous,
 				Inputs:    field.Inputs,
 			}
diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go
index fc6ac628b..9cfe4208d 100644
--- a/accounts/abi/abi_test.go
+++ b/accounts/abi/abi_test.go
@@ -832,9 +832,6 @@ func TestUnpackIntoMapNamingConflict(t *testing.T) {
 	if err = abi.UnpackIntoMap(receivedMap, "received", data); err != nil {
 		t.Error("naming conflict between two events; no error expected")
 	}
-	if len(receivedMap) != 1 {
-		t.Error("naming conflict between two events; event defined latest in the abi expected to be used")
-	}
 
 	// Method and event have the same name
 	abiJSON = `[{"constant":false,"inputs":[{"name":"memo","type":"bytes"}],"name":"received","outputs":[],"payable":true,"stateMutability":"payable","type":"function"},{"anonymous":false,"inputs":[{"indexed":false,"name":"sender","type":"address"},{"indexed":false,"name":"amount","type":"uint256"},{"indexed":false,"name":"memo","type":"bytes"}],"name":"received","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"sender","type":"address"}],"name":"receivedAddr","type":"event"}]`
@@ -999,3 +996,45 @@ func TestABI_EventById(t *testing.T) {
 		}
 	}
 }
+
+func TestDuplicateMethodNames(t *testing.T) {
+	abiJSON := `[{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"}],"name":"transfer","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"},{"name":"data","type":"bytes"}],"name":"transfer","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"},{"name":"data","type":"bytes"},{"name":"customFallback","type":"string"}],"name":"transfer","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"}]`
+	contractAbi, err := JSON(strings.NewReader(abiJSON))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := contractAbi.Methods["transfer"]; !ok {
+		t.Fatalf("Could not find original method")
+	}
+	if _, ok := contractAbi.Methods["transfer0"]; !ok {
+		t.Fatalf("Could not find duplicate method")
+	}
+	if _, ok := contractAbi.Methods["transfer1"]; !ok {
+		t.Fatalf("Could not find duplicate method")
+	}
+	if _, ok := contractAbi.Methods["transfer2"]; ok {
+		t.Fatalf("Should not have found extra method")
+	}
+}
+
+// TestDoubleDuplicateMethodNames checks that if transfer0 already exists, there won't be a name
+// conflict and that the second transfer method will be renamed transfer1.
+func TestDoubleDuplicateMethodNames(t *testing.T) {
+	abiJSON := `[{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"}],"name":"transfer","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"},{"name":"data","type":"bytes"}],"name":"transfer0","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"to","type":"address"},{"name":"value","type":"uint256"},{"name":"data","type":"bytes"},{"name":"customFallback","type":"string"}],"name":"transfer","outputs":[{"name":"ok","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"}]`
+	contractAbi, err := JSON(strings.NewReader(abiJSON))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := contractAbi.Methods["transfer"]; !ok {
+		t.Fatalf("Could not find original method")
+	}
+	if _, ok := contractAbi.Methods["transfer0"]; !ok {
+		t.Fatalf("Could not find duplicate method")
+	}
+	if _, ok := contractAbi.Methods["transfer1"]; !ok {
+		t.Fatalf("Could not find duplicate method")
+	}
+	if _, ok := contractAbi.Methods["transfer2"]; ok {
+		t.Fatalf("Should not have found extra method")
+	}
+}
diff --git a/accounts/abi/bind/base_test.go b/accounts/abi/bind/base_test.go
index f65c9e9b4..3ae685e00 100644
--- a/accounts/abi/bind/base_test.go
+++ b/accounts/abi/bind/base_test.go
@@ -345,29 +345,3 @@ func TestUnpackIndexedBytesTyLogIntoMap(t *testing.T) {
 		t.Error("unpacked map does not match expected map")
 	}
 }
-
-func TestUnpackIntoMapNamingConflict(t *testing.T) {
-	hash := crypto.Keccak256Hash([]byte("testName"))
-	mockLog := types.Log{
-		Address: common.HexToAddress("0x0"),
-		Topics: []common.Hash{
-			common.HexToHash("0x0"),
-			hash,
-		},
-		Data:        hexutil.MustDecode(hexData),
-		BlockNumber: uint64(26),
-		TxHash:      common.HexToHash("0x0"),
-		TxIndex:     111,
-		BlockHash:   common.BytesToHash([]byte{1, 2, 3, 4, 5}),
-		Index:       7,
-		Removed:     false,
-	}
-
-	abiString := `[{"anonymous":false,"inputs":[{"indexed":true,"name":"name","type":"string"},{"indexed":false,"name":"sender","type":"address"},{"indexed":false,"name":"amount","type":"uint256"},{"indexed":false,"name":"memo","type":"bytes"}],"name":"received","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"sender","type":"address"}],"name":"received","type":"event"}]`
-	parsedAbi, _ := abi.JSON(strings.NewReader(abiString))
-	bc := bind.NewBoundContract(common.HexToAddress("0x0"), parsedAbi, nil, nil, nil)
-	receivedMap := make(map[string]interface{})
-	if err := bc.UnpackLogIntoMap(receivedMap, "received", mockLog); err == nil {
-		t.Error("naming conflict between two events; error expected")
-	}
-}