Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/v1.52.0 #242

Merged
merged 2 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions client/chain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ type chainClient struct {

sessionEnabled bool

ofacChecker *OfacChecker

authQueryClient authtypes.QueryClient
authzQueryClient authztypes.QueryClient
bankQueryClient banktypes.QueryClient
Expand Down Expand Up @@ -440,22 +442,27 @@ func NewChainClient(
subaccountToNonce: make(map[ethcommon.Hash]uint32),
}

cc.ofacChecker, err = NewOfacChecker()
if err != nil {
return nil, errors.Wrap(err, "Error creating OFAC checker")
}
if cc.canSign {
var err error

cc.accNum, cc.accSeq, err = cc.txFactory.AccountRetriever().GetAccountNumberSequence(ctx, ctx.GetFromAddress())
account, err := cc.txFactory.AccountRetriever().GetAccount(ctx, ctx.GetFromAddress())
if err != nil {
err = errors.Wrap(err, "failed to get initial account num and seq")
err = errors.Wrapf(err, "failed to get account")
return nil, err
}

if cc.ofacChecker.IsBlacklisted(account.GetAddress().String()) {
return nil, errors.Errorf("Address %s is in the OFAC list", account.GetAddress())
}
cc.accNum, cc.accSeq = account.GetAccountNumber(), account.GetSequence()
go cc.runBatchBroadcast()
go cc.syncTimeoutHeight()
}

return cc, nil
}

func (c *chainClient) syncNonce() {
num, seq, err := c.txFactory.AccountRetriever().GetAccountNumberSequence(c.ctx, c.ctx.GetFromAddress())
if err != nil {
Expand Down Expand Up @@ -1153,6 +1160,9 @@ func (c *chainClient) GetAuthzGrants(ctx context.Context, req authztypes.QueryGr
}

func (c *chainClient) BuildGenericAuthz(granter, grantee, msgtype string, expireIn time.Time) *authztypes.MsgGrant {
if c.ofacChecker.IsBlacklisted(granter) {
panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
}
Comment on lines +1163 to +1165
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error instead of panicking.

The OFAC compliance check on the granter address is correctly performed. However, using panic is generally discouraged as it abruptly terminates the program. It would be better to return an error instead of panicking to allow proper error handling.

Apply this diff to return an error instead of panicking:

-	if c.ofacChecker.IsBlacklisted(granter) {
-		panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
-	}
+	if c.ofacChecker.IsBlacklisted(granter) {
+		return nil, errors.New("granter address is in the OFAC list")
+	}

Committable suggestion was skipped due to low confidence.

authz := authztypes.NewGenericAuthorization(msgtype)
authzAny := codectypes.UnsafePackAny(authz)
return &authztypes.MsgGrant{
Expand Down Expand Up @@ -1184,6 +1194,9 @@ var (
)

func (c *chainClient) BuildExchangeAuthz(granter, grantee string, authzType ExchangeAuthz, subaccountId string, markets []string, expireIn time.Time) *authztypes.MsgGrant {
if c.ofacChecker.IsBlacklisted(granter) {
panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
}
Comment on lines +1197 to +1199
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error instead of panicking.

The OFAC compliance check on the granter address is correctly performed. However, as mentioned in the previous comment, using panic is generally discouraged. It would be better to return an error instead of panicking to allow proper error handling.

Apply this diff to return an error instead of panicking:

-	if c.ofacChecker.IsBlacklisted(granter) {
-		panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
-	}
+	if c.ofacChecker.IsBlacklisted(granter) {
+		return nil, errors.New("granter address is in the OFAC list")
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if c.ofacChecker.IsBlacklisted(granter) {
panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
}
if c.ofacChecker.IsBlacklisted(granter) {
return nil, errors.New("granter address is in the OFAC list")
}

var typedAuthzAny codectypes.Any
var typedAuthzBytes []byte
switch authzType {
Expand Down Expand Up @@ -1279,6 +1292,9 @@ func (c *chainClient) BuildExchangeBatchUpdateOrdersAuthz(
derivativeMarkets []string,
expireIn time.Time,
) *authztypes.MsgGrant {
if c.ofacChecker.IsBlacklisted(granter) {
panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
}
Comment on lines +1295 to +1297
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error instead of panicking.

The OFAC compliance check on the granter address is correctly performed. However, as mentioned in the previous comments, using panic is generally discouraged. It would be better to return an error instead of panicking to allow proper error handling.

Apply this diff to return an error instead of panicking:

-	if c.ofacChecker.IsBlacklisted(granter) {
-		panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
-	}
+	if c.ofacChecker.IsBlacklisted(granter) {
+		return nil, errors.New("granter address is in the OFAC list")
+	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if c.ofacChecker.IsBlacklisted(granter) {
panic("Address is in the OFAC list") // panics should generally be avoided, but otherwise function signature should be changed
}
if c.ofacChecker.IsBlacklisted(granter) {
return nil, errors.New("granter address is in the OFAC list")
}

typedAuthz := &exchangetypes.BatchUpdateOrdersAuthz{
SubaccountId: subaccountId,
SpotMarkets: spotMarkets,
Expand Down
71 changes: 70 additions & 1 deletion client/chain/chain_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package chain

import (
"encoding/json"
"io"
"os"
"testing"

"github.com/stretchr/testify/suite"

"github.com/InjectiveLabs/sdk-go/client"
"github.com/InjectiveLabs/sdk-go/client/common"
rpchttp "github.com/cometbft/cometbft/rpc/client/http"
Expand Down Expand Up @@ -51,6 +55,72 @@ func createClient(senderAddress cosmtypes.AccAddress, cosmosKeyring keyring.Keyr
return chainClient, err
}

type OfacTestSuite struct {
suite.Suite
network common.Network
tmClient *rpchttp.HTTP
senderAddress cosmtypes.AccAddress
cosmosKeyring keyring.Keyring
}

func (suite *OfacTestSuite) SetupTest() {
var err error
suite.network = common.LoadNetwork("testnet", "lb")
suite.tmClient, err = rpchttp.New(suite.network.TmEndpoint, "/websocket")
suite.NoError(err)

suite.senderAddress, suite.cosmosKeyring, err = accountForTests()
suite.NoError(err)

// Prepare OFAC list file
testList := []string{
suite.senderAddress.String(),
}
jsonData, err := json.Marshal(testList)
suite.NoError(err)

ofacListFilename = "ofac_test.json"
file, err := os.Create(getOfacListPath())
suite.NoError(err)

_, err = io.WriteString(file, string(jsonData))
suite.NoError(err)

err = file.Close()
suite.NoError(err)
}

func (suite *OfacTestSuite) TearDownTest() {
err := os.Remove(getOfacListPath())
suite.NoError(err)
ofacListFilename = defaultofacListFilename
}

func (suite *OfacTestSuite) TestOfacList() {
clientCtx, err := NewClientContext(
suite.network.ChainId,
suite.senderAddress.String(),
suite.cosmosKeyring,
)
suite.NoError(err)

clientCtx = clientCtx.WithNodeURI(suite.network.TmEndpoint).WithClient(suite.tmClient)
testChecker, err := NewOfacChecker()
suite.NoError(err)
suite.Equal(1, len(testChecker.ofacList))

_, err = NewChainClient(
clientCtx,
suite.network,
common.OptionGasPrices(client.DefaultGasPriceWithDenom),
)
suite.Error(err)
}

func TestOfacTestSuite(t *testing.T) {
suite.Run(t, new(OfacTestSuite))
}

func TestDefaultSubaccount(t *testing.T) {
network := common.LoadNetwork("devnet", "lb")
senderAddress, cosmosKeyring, err := accountForTests()
Expand Down Expand Up @@ -103,5 +173,4 @@ func TestGetSubaccountWithIndex(t *testing.T) {
if subaccountThirty != expectedSubaccountThirtyIdHash {
t.Error("The subaccount with index 30 was calculated incorrectly")
}

}
97 changes: 97 additions & 0 deletions client/chain/ofac.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package chain

import (
"encoding/json"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"strings"
)

const (
defaultOfacListURL = "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/master/wallets/ofac.json"
defaultofacListFilename = "ofac.json"
)

var (
ofacListFilename = defaultofacListFilename
)

type OfacChecker struct {
ofacListPath string
ofacList map[string]bool
}

func NewOfacChecker() (*OfacChecker, error) {
checker := &OfacChecker{
ofacListPath: getOfacListPath(),
}
if _, err := os.Stat(checker.ofacListPath); os.IsNotExist(err) {
if err := DownloadOfacList(); err != nil {
return nil, err
}
}
if err := checker.loadOfacList(); err != nil {
return nil, err
}
return checker, nil
}

func getOfacListPath() string {
currentDirectory, _ := os.Getwd()
for !strings.HasSuffix(currentDirectory, "sdk-go") {
currentDirectory = filepath.Dir(currentDirectory)
}
return filepath.Join(filepath.Join(filepath.Join(currentDirectory, "client"), "metadata"), ofacListFilename)
}
Comment on lines +42 to +48
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider improving the OFAC list path determination.

The current implementation assumes a specific directory structure and may break if the structure changes. It also doesn't handle the case where the "sdk-go" directory is not found.

Consider the following improvements:

  • Use a configuration file or environment variable to specify the OFAC list path instead of relying on a specific directory structure. This would make the code more flexible and easier to maintain.
  • Handle the case where the "sdk-go" directory is not found by returning an error or using a default path. This would make the code more robust and prevent potential issues.


func DownloadOfacList() error {
resp, err := http.Get(defaultOfacListURL)
if err != nil {
return err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return fmt.Errorf("failed to download OFAC list, status code: %d", resp.StatusCode)
}

outFile, err := os.Create(getOfacListPath())
if err != nil {
return err
}
defer outFile.Close()

_, err = io.Copy(outFile, resp.Body)
if err != nil {
return err
}
_, err = outFile.WriteString("\n")
if err != nil {
return err
}
return nil
}

func (oc *OfacChecker) loadOfacList() error {
file, err := os.ReadFile(oc.ofacListPath)
if err != nil {
return err
}
var list []string
err = json.Unmarshal(file, &list)
if err != nil {
return err
}
oc.ofacList = make(map[string]bool)
for _, item := range list {
oc.ofacList[item] = true
}
return nil
}

func (oc *OfacChecker) IsBlacklisted(address string) bool {
return oc.ofacList[address]
}
48 changes: 48 additions & 0 deletions client/metadata/ofac.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
[
"0x179f48c78f57a3a78f0608cc9197b8972921d1d2",
"0x1967d8af5bd86a497fb3dd7899a020e47560daaf",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x1da5821544e25c636c1417ba96ade4cf6d2f9b5a",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f50508a8a3d323b91336fa3ea6ae50e55f32185",
"0x308ed4b7b49797e1a98d3818bff6fe5385410370",
"0x3cbded43efdaf0fc77b9c55f6fc9988fcc9b757d",
"0x3efa30704d2b8bbac821307230376556cf8cc39e",
"0x48549a34ae37b12f6a30566245176994e17c6b4a",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x530a64c0ce595026a4a556b703644228179e2d57",
"0x5512d943ed1f7c8a43f3435c85f7ab68b30121b0",
"0x5a7a51bfb49f190e5a6060a5bc6052ac14a3b59f",
"0x5f48c2a71b2cc96e3f0ccae4e39318ff0dc375b2",
"0x6be0ae71e6c41f2f9d0d1a3b8d0f75e6f6a0b46e",
"0x6f1ca141a28907f78ebaa64fb83a9088b02a8352",
"0x746aebc06d2ae31b71ac51429a19d54e797878e9",
"0x77777feddddffc19ff86db637967013e6c6a116c",
"0x797d7ae72ebddcdea2a346c1834e04d1f8df102b",
"0x8576acc5c05d6ce88f4e49bf65bdf0c62f91353c",
"0x901bb9583b24d97e995513c6778dc6888ab6870e",
"0x961c5be54a2ffc17cf4cb021d863c42dacd47fc1",
"0x97b1043abd9e6fc31681635166d430a458d14f9c",
"0x9c2bc757b66f24d60f016b6237f8cdd414a879fa",
"0x9f4cda013e354b8fc285bf4b9a60460cee7f7ea9",
"0xa7e5d5a720f06526557c513402f2e6b5fa20b008",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xc455f7fd3e0e12afd51fba5c106909934d8a0e4a",
"0xca0840578f57fe71599d29375e16783424023357",
"0xd0975b32cea532eadddfc9c60481976e39db3472",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xe1d865c3d669dcc8c57c8d023140cb204e672ee4",
"0xe7aa314c77f4233c18c6cc84384a9247c0cf367b",
"0xed6e0a7e4ac94d976eebfb82ccf777a3c6bad921",
"0xf3701f445b6bdafedbca97d1e477357839e4120d",
"0xfac583c0cf07ea434052c49115a4682172ab6b4f",
"0xfec8a60023265364d066a1212fde3930f6ae8da7",
"0xffbac21a641dcfe4552920138d90f3638b3c9fba"
]
12 changes: 12 additions & 0 deletions examples/chain/ofac/1_DownloadOfacList/example.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package main

import (
chainclient "github.com/InjectiveLabs/sdk-go/client/chain"
)

func main() {
err := chainclient.DownloadOfacList()
if err != nil {
panic(err)
}
}
Loading