Sitelet https://github.com/ethereum-optimism/optimism/commit/4037c8a2a7fdc99e7c23ca435eaf780cd2a64592
Skip to content

Commit 4037c8a

Browse files
smartcontractssmartcontracts
authored
feat: improve error reporting for interface check (#13924)
Improves error reporting for the interface check by having it detail exactly what you need to do.
1 parent 5d2d03f commit 4037c8a

2 files changed

Lines changed: 105 additions & 83 deletions

File tree

‎packages/contracts-bedrock/scripts/checks/interfaces/main.go‎

Lines changed: 105 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@ import (
77
"log"
88
"os"
99
"path/filepath"
10-
"sort"
1110
"strings"
1211

1312
"github.com/ethereum-optimism/optimism/packages/contracts-bedrock/scripts/checks/common"
14-
"github.com/google/go-cmp/cmp"
1513
)
1614

1715
var excludeContracts = []string{
@@ -130,7 +128,7 @@ func processFile(artifactPath string) (*common.Void, []error) {
130128
return nil, []error{fmt.Errorf("failed to compare ABIs: %w", err)}
131129
}
132130
if !match {
133-
return nil, []error{fmt.Errorf("%s: Differences found in ABI between interface and actual contract", contractName)}
131+
return nil, []error{fmt.Errorf("differences found")}
134132
}
135133

136134
return nil, nil
@@ -229,53 +227,115 @@ func normalizeInternalType(internalType string) string {
229227
}
230228

231229
func compareABIs(abi1, abi2 json.RawMessage) (bool, error) {
232-
var data1, data2 []map[string]interface{}
230+
var interfaceABI, contractABI []map[string]interface{}
233231

234-
if err := json.Unmarshal(abi1, &data1); err != nil {
235-
return false, fmt.Errorf("error unmarshalling first ABI: %w", err)
232+
if err := json.Unmarshal(abi1, &interfaceABI); err != nil {
233+
return false, fmt.Errorf("error unmarshalling interface ABI: %w", err)
236234
}
237235

238-
if err := json.Unmarshal(abi2, &data2); err != nil {
239-
return false, fmt.Errorf("error unmarshalling second ABI: %w", err)
236+
if err := json.Unmarshal(abi2, &contractABI); err != nil {
237+
return false, fmt.Errorf("error unmarshalling contract ABI: %w", err)
240238
}
241239

242-
// Sort the ABI data
243-
sort.Slice(data1, func(i, j int) bool {
244-
return abiItemLess(data1[i], data1[j])
245-
})
246-
sort.Slice(data2, func(i, j int) bool {
247-
return abiItemLess(data2[i], data2[j])
248-
})
240+
// Create maps for easier lookup
241+
interfaceItems := make(map[string]map[string]interface{})
242+
contractItems := make(map[string]map[string]interface{})
243+
244+
// Helper to create a unique key for each ABI item
245+
makeKey := func(item map[string]interface{}) string {
246+
itemType := getString(item, "type")
247+
itemName := getString(item, "name")
248+
inputs, _ := json.Marshal(item["inputs"])
249+
outputs, _ := json.Marshal(item["outputs"])
250+
return fmt.Sprintf("%s_%s_%s_%s", itemType, itemName, inputs, outputs)
251+
}
249252

250-
// Compare using go-cmp
251-
diff := cmp.Diff(data1, data2)
252-
if diff != "" {
253-
log.Printf("ABI diff: %s", diff)
254-
return false, nil
253+
// Build lookup maps
254+
for _, item := range interfaceABI {
255+
key := makeKey(item)
256+
interfaceItems[key] = item
257+
}
258+
for _, item := range contractABI {
259+
key := makeKey(item)
260+
contractItems[key] = item
255261
}
256-
return true, nil
257-
}
258262

259-
func abiItemLess(a, b map[string]interface{}) bool {
260-
aType := getString(a, "type")
261-
bType := getString(b, "type")
263+
// Check for missing items in both directions
264+
isMatch := true
265+
266+
// Check interface items exist in contract
267+
for key, item := range interfaceItems {
268+
if _, exists := contractItems[key]; !exists {
269+
itemType := getString(item, "type")
270+
signature := formatABIItem(item)
271+
log.Printf("REMOVE %s from interface: %s", itemType, signature)
272+
isMatch = false
273+
}
274+
}
262275

263-
if aType != bType {
264-
return aType < bType
276+
// Check contract items exist in interface
277+
for key, item := range contractItems {
278+
if _, exists := interfaceItems[key]; !exists {
279+
itemType := getString(item, "type")
280+
signature := formatABIItem(item)
281+
log.Printf("ADD %s to interface: %s", itemType, signature)
282+
isMatch = false
283+
}
265284
}
266285

267-
aName := getString(a, "name")
268-
bName := getString(b, "name")
269-
return aName < bName
286+
return isMatch, nil
270287
}
271288

272-
func getString(m map[string]interface{}, key string) string {
273-
if v, ok := m[key]; ok {
274-
if s, ok := v.(string); ok {
275-
return s
289+
// Helper function to format ABI item into a readable signature
290+
func formatABIItem(item map[string]interface{}) string {
291+
itemType := getString(item, "type")
292+
itemName := getString(item, "name")
293+
294+
// Format inputs
295+
inputs, _ := item["inputs"].([]interface{})
296+
inputStr := make([]string, 0, len(inputs))
297+
for _, input := range inputs {
298+
if inputMap, ok := input.(map[string]interface{}); ok {
299+
paramType := getString(inputMap, "type")
300+
paramName := getString(inputMap, "name")
301+
if paramName != "" {
302+
inputStr = append(inputStr, fmt.Sprintf("%s %s", paramType, paramName))
303+
} else {
304+
inputStr = append(inputStr, paramType)
305+
}
276306
}
277307
}
278-
return ""
308+
309+
// Format outputs
310+
outputs, _ := item["outputs"].([]interface{})
311+
outputStr := make([]string, 0, len(outputs))
312+
for _, output := range outputs {
313+
if outputMap, ok := output.(map[string]interface{}); ok {
314+
paramType := getString(outputMap, "type")
315+
paramName := getString(outputMap, "name")
316+
if paramName != "" {
317+
outputStr = append(outputStr, fmt.Sprintf("%s %s", paramType, paramName))
318+
} else {
319+
outputStr = append(outputStr, paramType)
320+
}
321+
}
322+
}
323+
324+
// Build the signature based on the item type
325+
switch itemType {
326+
case "function":
327+
returnStr := ""
328+
if len(outputStr) > 0 {
329+
returnStr = fmt.Sprintf(" returns (%s)", strings.Join(outputStr, ", "))
330+
}
331+
return fmt.Sprintf("function %s(%s)%s", itemName, strings.Join(inputStr, ", "), returnStr)
332+
case "event":
333+
return fmt.Sprintf("event %s(%s)", itemName, strings.Join(inputStr, ", "))
334+
case "constructor":
335+
return fmt.Sprintf("constructor(%s)", strings.Join(inputStr, ", "))
336+
default:
337+
return fmt.Sprintf("%s %s(%s)", itemType, itemName, strings.Join(inputStr, ", "))
338+
}
279339
}
280340

281341
func isExcluded(contractName string) bool {
@@ -286,3 +346,13 @@ func isExcluded(contractName string) bool {
286346
}
287347
return false
288348
}
349+
350+
// getString safely retrieves a string value from a map[string]interface{}
351+
func getString(m map[string]interface{}, key string) string {
352+
if val, ok := m[key]; ok {
353+
if str, ok := val.(string); ok {
354+
return str
355+
}
356+
}
357+
return ""
358+
}

‎packages/contracts-bedrock/scripts/checks/interfaces/main_test.go‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -245,51 +245,3 @@ func TestNormalizeInternalType(t *testing.T) {
245245
})
246246
}
247247
}
248-
249-
func TestABIItemLess(t *testing.T) {
250-
tests := []struct {
251-
name string
252-
a map[string]interface{}
253-
b map[string]interface{}
254-
want bool
255-
}{
256-
{
257-
name: "Different types",
258-
a: map[string]interface{}{"type": "constructor"},
259-
b: map[string]interface{}{"type": "function"},
260-
want: true,
261-
},
262-
{
263-
name: "Same type, different names",
264-
a: map[string]interface{}{"type": "function", "name": "a"},
265-
b: map[string]interface{}{"type": "function", "name": "b"},
266-
want: true,
267-
},
268-
{
269-
name: "Same type and name",
270-
a: map[string]interface{}{"type": "function", "name": "test"},
271-
b: map[string]interface{}{"type": "function", "name": "test"},
272-
want: false,
273-
},
274-
{
275-
name: "Constructor vs function",
276-
a: map[string]interface{}{"type": "constructor"},
277-
b: map[string]interface{}{"type": "function", "name": "test"},
278-
want: true,
279-
},
280-
{
281-
name: "Event vs function",
282-
a: map[string]interface{}{"type": "event", "name": "TestEvent"},
283-
b: map[string]interface{}{"type": "function", "name": "test"},
284-
want: true,
285-
},
286-
}
287-
288-
for _, tt := range tests {
289-
t.Run(tt.name, func(t *testing.T) {
290-
if got := abiItemLess(tt.a, tt.b); got != tt.want {
291-
t.Errorf("abiItemLess() = %v, want %v", got, tt.want)
292-
}
293-
})
294-
}
295-
}

0 commit comments

Comments
 (0)