Repository: NethermindEth/juno
Pull request #147 (2022-05-10 21:20:57)

Remove error returns from the ABI manager
## Changes:
- Remove error returns in the `PutABI` and `GetABI` methods

## Types of changes

What types of changes does your code introduce?
_Put an `x` in the boxes that apply_

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation Update
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Other (please describe): 

## Testing
**Requires testing**

- [x] Yes
- [ ] No

**In case you checked yes, did you write tests??**

- [x] Yes
- [ ] No


codecov[bot]: # [Codecov](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) Report
> Merging [#147](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) (68bf91a) into [main](https://codecov.io/gh/NethermindEth/juno/commit/7797be84d98fc51d4611f0f735e383fd77053be3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) (7797be8) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff            @@
##              main      #147   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines         1446      1441    -5     
=========================================
- Hits          1446      1441    -5     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `100.00% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth) | Coverage Δ | |
|---|---|---|
| [pkg/db/abi/manager.go](https://codecov.io/gh/NethermindEth/juno/pull/147/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL2RiL2FiaS9tYW5hZ2VyLmdv) | `100.00% <100.00%> (ø)` | |
| [pkg/crypto/weierstrass/weierstrass.go](https://codecov.io/gh/NethermindEth/juno/pull/147/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth#diff-cGtnL2NyeXB0by93ZWllcnN0cmFzcy93ZWllcnN0cmFzcy5nbw==) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth). Last update [7797be8...68bf91a](https://codecov.io/gh/NethermindEth/juno/pull/147?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NethermindEth).


Patch:
diff --git a/pkg/db/abi/manager.go b/pkg/db/abi/manager.go
index adc88fe0..0426c4e7 100644
--- a/pkg/db/abi/manager.go
+++ b/pkg/db/abi/manager.go
@@ -29,12 +29,9 @@ func NewABIManager(database db.Databaser) *Manager {
 // must be a hexadecimal string without the 0x prefix, if the contract address encoding
 // is invalid then an InvalidContractAddress error is returned. If the ABI does
 // not exist, then returns nil without error.
-func (m *Manager) GetABI(contractAddress string) (*Abi, error) {
+func (m *Manager) GetABI(contractAddress string) *Abi {
 	// Build the key from contract address
-	key, err := buildKey(contractAddress)
-	if err != nil {
-		return nil, err
-	}
+	key := buildKey(contractAddress)
 	// Query to database
 	data, err := m.database.Get(key)
 	if err != nil {
@@ -42,7 +39,7 @@ func (m *Manager) GetABI(contractAddress string) (*Abi, error) {
 		panic(any(fmt.Errorf("%w: %s", DbError, err)))
 	}
 	if data == nil {
-		return nil, nil
+		return nil
 	}
 	// Unmarshal the data from database
 	abi := new(Abi)
@@ -50,18 +47,15 @@ func (m *Manager) GetABI(contractAddress string) (*Abi, error) {
 		// notest
 		panic(any(fmt.Errorf("%w: %s", UnmarshalError, err.Error())))
 	}
-	return abi, nil
+	return abi
 }
 
 // PutABI puts the ABI to the contract address. The contract address must be a
 // hexadecimal string without the 0x prefix, if the contract address is invalid
 // then an InvalidContractAddress error is returned.
-func (m *Manager) PutABI(contractAddress string, abi *Abi) (err error) {
+func (m *Manager) PutABI(contractAddress string, abi *Abi) {
 	// Build the key from contract address
-	key, err := buildKey(contractAddress)
-	if err != nil {
-		return err
-	}
+	key := buildKey(contractAddress)
 	value, err := json.Marshal(abi)
 	if err != nil {
 		// notest
@@ -72,13 +66,12 @@ func (m *Manager) PutABI(contractAddress string, abi *Abi) (err error) {
 		// notest
 		panic(any(fmt.Errorf("%w: %s", DbError, err.Error())))
 	}
-	return nil
 }
 
-func buildKey(contractAddress string) ([]byte, error) {
+func buildKey(contractAddress string) []byte {
 	address, ok := new(big.Int).SetString(contractAddress, 16)
 	if !ok {
-		return nil, fmt.Errorf("%w: %s", InvalidContractAddress, contractAddress)
+		panic(fmt.Errorf("%w: %s", InvalidContractAddress, contractAddress))
 	}
-	return address.Bytes(), nil
+	return address.Bytes()
 }


Test patch:
diff --git a/pkg/db/abi/manager_test.go b/pkg/db/abi/manager_test.go
index 7d0707f3..7a8f8e51 100644
--- a/pkg/db/abi/manager_test.go
+++ b/pkg/db/abi/manager_test.go
@@ -3,7 +3,6 @@ package abi
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"github.com/NethermindEth/juno/pkg/db"
 	"testing"
 )
@@ -59,14 +58,8 @@ func TestManager(t *testing.T) {
 	}
 	// Run all tests
 	for _, test := range tests {
-		err := manager.PutABI(test.ContractAddress, &test.Abi)
-		if err != nil {
-			t.Error(err)
-		}
-		abi, err := manager.GetABI(test.ContractAddress)
-		if err != nil {
-			t.Error(err)
-		}
+		manager.PutABI(test.ContractAddress, &test.Abi)
+		abi := manager.GetABI(test.ContractAddress)
 		raw1, err := json.Marshal(&test.Abi)
 		if err != nil {
 			t.Error(err)
@@ -81,33 +74,11 @@ func TestManager(t *testing.T) {
 	}
 }
 
-func TestManager_PutABI_InvalidAddress(t *testing.T) {
-	// Init the ABI manager
-	database := db.New(t.TempDir(), 0)
-	manager := NewABIManager(database)
-	err := manager.PutABI("0x1bd7ca87f139693e6681be2042194cf631c4e8d77027bf0ea9e6d55fc6018ac", &Abi{})
-	if !errors.Is(err, InvalidContractAddress) {
-		t.Errorf("returned error %s, want: %s", err, InvalidContractAddress)
-	}
-}
-
-func TestManager_GetABI_InvalidAddress(t *testing.T) {
-	// Init the ABI manager
-	database := db.New(t.TempDir(), 0)
-	manager := NewABIManager(database)
-	_, err := manager.GetABI("0x1bd7ca87f139693e6681be2042194cf631c4e8d77027bf0ea9e6d55fc6018ac")
-	if !errors.Is(err, InvalidContractAddress) {
-		t.Errorf("returned error %s, want: %s", err, InvalidContractAddress)
-	}
-}
 func TestManager_GetABI_NotFound(t *testing.T) {
 	// Init the ABI manager
 	database := db.New(t.TempDir(), 0)
 	manager := NewABIManager(database)
-	abi, err := manager.GetABI("1bd7ca87f139693e6681be2042194cf631c4e8d77027bf0ea9e6d55fc6018ac")
-	if err != nil {
-		t.Errorf("unexpected error %s", err)
-	}
+	abi := manager.GetABI("1bd7ca87f139693e6681be2042194cf631c4e8d77027bf0ea9e6d55fc6018ac")
 	if abi != nil {
 		t.Errorf("abi must be nil")
 	}