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.

Sign up for GitHub

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

Feat/gardenv1 #6

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Feat/gardenv1 #6

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
9230d71
add services for v1 stack
yash1io Aug 9, 2024
b8ee0ce
updated to use garden watchers
yash1io Sep 4, 2024
afde747
remove binary
yash1io Sep 4, 2024
87f0169
Merge branch 'main' into feat/gardenv1
yash1io Sep 9, 2024
a122d18
add bit ponder
yash1io Sep 19, 2024
9d6f8a2
add cobiv2 support
Revantark Sep 22, 2024
b49a426
fund cobiv2 addresses
Revantark Sep 23, 2024
c2f2b26
update cobi evm address
Revantark Sep 23, 2024
9dbb181
add relayer support
Revantark Sep 26, 2024
68810df
update relayer docker config
Revantark Oct 1, 2024
d4e4a5f
resolved conflicts with main
Oct 3, 2024
6cfbca1
add support for quote server
Revantark Oct 13, 2024
a965b4e
fix fund
Revantark Oct 21, 2024
bcfaf94
add solana validator
Revantark Nov 18, 2024
8dfe3be
add solana services
Revantark Nov 20, 2024
bd2c85e
Update ports for solana validator, relayer
harshasingamshetty1 Nov 21, 2024
65592ac
feat/faucet: Add Solana support (#7)
skr4n Nov 21, 2024
c8a55d5
add bit-indexer
Revantark Dec 26, 2024
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
20 changes: 18 additions & 2 deletions fund.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/chaincfg"
"github.com/catalogfi/blockchain"
"github.com/catalogfi/blockchain/localnet"
"github.com/ethereum/go-ethereum/common"
)
Expand Down Expand Up @@ -40,6 +41,8 @@ func (m *Merry) Fund(to string) error {

func fundEVM(to string) error {
ethAmount, _ := new(big.Int).SetString("1000000000000000000", 10)
seedAmount, _ := new(big.Int).SetString("1000000000000000000", 10)

wbtcAmount, _ := new(big.Int).SetString("100000000", 10)
wallet, err := localnet.EVMWallet(0)
if err != nil {
Expand All @@ -49,8 +52,11 @@ func fundEVM(to string) error {
if err != nil {
return fmt.Errorf("failed to send eth: %v", err)
}

ethereumWBTCAsset := blockchain.NewERC20(blockchain.NewEvmChain(blockchain.EthereumLocalnet), common.HexToAddress("0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512"), common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0"))

fmt.Printf("Successfully sent %v ETH on Ethereum Localnet at: http://localhost:5100/tx/%s\n", ethAmount, tx.Hash().Hex())
tx2, err := wallet.Send(context.Background(), localnet.WBTC(), common.HexToAddress(to), wbtcAmount)
tx2, err := wallet.Send(context.Background(), ethereumWBTCAsset, common.HexToAddress(to), wbtcAmount)
if err != nil {
return fmt.Errorf("failed to send eth: %v", err)
}
Comment on lines +75 to 78
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Error messages for WBTC transactions reference wrong asset

In the error handling after sending WBTC (tx2 and tx4), the error messages incorrectly state "failed to send eth" instead of "failed to send WBTC". This inconsistency might lead to confusion during error troubleshooting.

Apply this diff to correct the error messages:

// After tx2 (Ethereum Localnet WBTC transaction)
	if err != nil {
-		return fmt.Errorf("failed to send eth: %v", err)
+		return fmt.Errorf("failed to send WBTC: %v", err)
	}

// After tx4 (Arbitrum Localnet WBTC transaction)
	if err != nil {
-		return fmt.Errorf("failed to send eth: %v", err)
+		return fmt.Errorf("failed to send WBTC: %v", err)
	}

Also applies to: 71-74

Expand All @@ -59,12 +65,22 @@ func fundEVM(to string) error {
if err != nil {
return fmt.Errorf("failed to send eth: %v", err)
}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect amount displayed in log message

In the fmt.Printf statement logging the ETH transaction on Arbitrum Localnet, wbtcAmount is used instead of ethAmount. This may lead to incorrect information being displayed to the user.

Apply this diff to correct the displayed amount:

	fmt.Printf("Successfully sent %v ETH on Arbitrum Localnet at: http://localhost:5101/tx/%s\n",
-		wbtcAmount, tx3.Hash().Hex())
+		ethAmount, tx3.Hash().Hex())

Committable suggestion was skipped due to low confidence.

fmt.Printf("Successfully sent %v ETH on Arbitrum Localnet at: http://localhost:5101/tx/%s\n", wbtcAmount, tx3.Hash().Hex())
tx4, err := wallet.Send(context.Background(), localnet.ArbitrumWBTC(), common.HexToAddress(to), wbtcAmount)
arbWBTCAsset := blockchain.NewERC20(blockchain.NewEvmChain(blockchain.ArbitrumLocalnet), common.HexToAddress("0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0"), common.HexToAddress("0x0165878A594ca255338adfa4d48449f69242Eb8F"))
tx4, err := wallet.Send(context.Background(), arbWBTCAsset, common.HexToAddress(to), wbtcAmount)
if err != nil {
return fmt.Errorf("failed to send eth: %v", err)
}
fmt.Printf("Successfully sent %v WBTC on Arbitrum Localnet at: http://localhost:5101/tx/%s\n", wbtcAmount, tx4.Hash().Hex())

arbSeedAsset := blockchain.NewERC20(blockchain.NewEvmChain(blockchain.ArbitrumLocalnet), common.HexToAddress("0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512"), common.HexToAddress("0x5FC8d32690cc91D4c39d9d3abcBD16989F875707"))
tx5, err := wallet.Send(context.Background(), arbSeedAsset, common.HexToAddress(to), wbtcAmount)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect amount used when sending SEED tokens

The wallet.Send function is using wbtcAmount instead of seedAmount when sending SEED tokens (arbSeedAsset). This might result in sending the wrong amount of SEED tokens.

Apply this diff to use seedAmount:

	tx5, err := wallet.Send(context.Background(), arbSeedAsset, common.HexToAddress(to),
-		wbtcAmount)
+		seedAmount)
📝 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
tx5, err := wallet.Send(context.Background(), arbSeedAsset, common.HexToAddress(to), wbtcAmount)
tx5, err := wallet.Send(context.Background(), arbSeedAsset, common.HexToAddress(to), seedAmount)

if err != nil {
return fmt.Errorf("failed to send eth: %v", err)
}
Comment on lines +95 to +97
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inconsistent error handling messages

Several error messages within different transaction blocks use the same generic message "failed to send eth", even when sending different assets like WBTC or SEED tokens. Consistent and specific error messages improve debuggability.

Consider updating all error messages to reflect the specific asset involved in the transaction.


fmt.Printf("Successfully sent %v SEED on Arbitrum Localnet at: http://localhost:5101/tx/%s\n", seedAmount, tx5.Hash().Hex())
return nil
}

Expand Down
13 changes: 10 additions & 3 deletions go.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (m *Merry) Start() error {
}
composePath := filepath.Join(home, ".merry", "docker-compose.yml")

bashCmd := runDockerCompose(composePath, "up", "-d", "cobi", "esplora", "ethereum-explorer", "arbitrum-explorer", "nginx")
bashCmd := runDockerCompose(composePath, "up", "-d", "cobi", "esplora", "ethereum-explorer", "arbitrum-explorer", "nginx", "garden-evm-watcher", "garden-db", "quote", "bit-ponder", "cobiv2", "relayer", "solana-validator", "virtual-balance", "solana-executor", "solana-relayer", "solana-watcher")
if m.IsHeadless && m.IsBare {
bashCmd = runDockerCompose(composePath, "up", "-d", "chopsticks", "ethereum", "arbitrum", "cosigner")
} else if m.IsHeadless {
Expand All @@ -39,7 +39,7 @@ func (m *Merry) Start() error {
fmt.Println("ENDPOINTS")
for name, endpoint := range m.Services {
if m.IsBare {
if name == "cobi" || name == "redis" || name == "orderbook" || name == "postgres" {
if name == "cobi" || name == "redis" || name == "orderbook" || name == "postgres" || name == "garden-evm-watcher" || name == "garden-db" || name == "matcher" || name == "bit-ponder" {
continue
}
Comment on lines +42 to 44
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor service exclusion logic for maintainability

The condition to exclude services is becoming lengthy and harder to maintain:

if name == "cobi" || name == "redis" || name == "orderbook" || name == "postgres" || name == "garden-evm-watcher" || name == "garden-db" || name == "matcher" || name == "bit-ponder" {
	continue
}

Consider refactoring this by using a slice or map to store the service names and check for inclusion. This approach enhances readability and makes it easier to manage the list of services.

Example using a slice:

excludedServices := []string{"cobi", "redis", "orderbook", "postgres", "garden-evm-watcher", "garden-db", "matcher", "bit-ponder"}

if contains(excludedServices, name) {
	continue
}

func contains(slice []string, item string) bool {
	for _, s := range slice {
		if s == item {
			return true
		}
	}
	return false
}

}
Expand All @@ -57,8 +57,15 @@ func (m *Merry) Start() error {
}

retry(func() error {
return fundBTC("bcrt1q5428vq2uzwhm3taey9sr9x5vm6tk78ew8pf2xw")
// cobi btc address
return fundBTC("bcrt1qgyf47wrtnr9gsr06gn62ft6m4lzylcnllrf9cf")
})

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid hardcoding Bitcoin addresses

The Bitcoin address is hardcoded in the fundBTC function call:

return fundBTC("bcrt1qgyf47wrtnr9gsr06gn62ft6m4lzylcnllrf9cf")

For better maintainability and security, consider moving the address to a configuration file, environment variable, or constant. This makes it easier to update the address without modifying the code.

retry(func() error {
// cobi evm addresss
return fundEVM("0x70997970c51812dc3a010c7d01b50e0d17dc79c8")
})

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct typo and avoid hardcoding Ethereum addresses

There is a typo in the comment:

// cobi evm addresss
  • "addresss" should be "address".

Additionally, the Ethereum address is hardcoded in the fundEVM function call:

return fundEVM("0x70997970c51812dc3a010c7d01b50e0d17dc79c8")

Similar to the Bitcoin address, consider moving this address to a configuration file, environment variable, or constant to enhance maintainability and security.

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance retry mechanism with timeout and max attempts

The current retry implementation could potentially run indefinitely. Consider adding:

  1. Maximum retry attempts
  2. Exponential backoff
  3. Timeout mechanism

Here's a suggested implementation:

+func retryWithTimeout(operation func() error, maxAttempts int, timeout time.Duration) error {
+    var lastErr error
+    attempt := 0
+    start := time.Now()
+
+    for attempt < maxAttempts {
+        if time.Since(start) > timeout {
+            return fmt.Errorf("operation timed out after %v: %v", timeout, lastErr)
+        }
+
+        if err := operation(); err == nil {
+            return nil
+        } else {
+            lastErr = err
+            attempt++
+            time.Sleep(time.Second * time.Duration(math.Pow(2, float64(attempt))))
+        }
+    }
+    return fmt.Errorf("operation failed after %d attempts: %v", maxAttempts, lastErr)
+}

-    retry(func() error {
-        return fundBTC("bcrt1qgyf47wrtnr9gsr06gn62ft6m4lzylcnllrf9cf")
-    })
+    if err := retryWithTimeout(func() error {
+        return fundBTC("bcrt1qgyf47wrtnr9gsr06gn62ft6m4lzylcnllrf9cf")
+    }, 5, time.Minute*2); err != nil {
+        return fmt.Errorf("failed to fund BTC: %v", err)
+    }

Committable suggestion skipped: line range outside the PR's diff.

retry(func() error {
// try establishing connection with the ethereum clients
_, err := localnet.EVMClient()
Expand Down
122 changes: 117 additions & 5 deletions resources/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ services:
arbitrum:
image: ghcr.io/catalogfi/garden_sol:latest
container_name: arbitrum
environment:
environment:
CHAIN_ID: 31338
LIGHT: "false"
ports:
- 8546:8545

arbitrum-explorer:
image: otterscan/otterscan:latest
container_name: arbitrum-explorer
Expand All @@ -35,10 +35,30 @@ services:
ports:
- 5101:80

relayer:
image: ghcr.io/catalogfi/relayer:latest
container_name: relayer
restart: unless-stopped
depends_on:
- garden-evm-watcher
- matcher
ports:
- 4426:4426

quote:
image: ghcr.io/catalogfi/quote:latest
container_name: quote
restart: unless-stopped
depends_on:
- garden-evm-watcher
- matcher
ports:
- 6969:6969

ethereum:
image: ghcr.io/catalogfi/garden_sol:latest
container_name: ethereum
environment:
environment:
CHAIN_ID: 31337
LIGHT: "true"
ports:
Comment on lines +61 to 64
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Clarification Needed on LIGHT Settings

I've noticed that the LIGHT environment variable is set to "true" for the ethereum service and "false" for another service. Could you please explain the reasoning behind these differing settings? This discrepancy might influence the behavior and performance of the respective services.

🔗 Analysis chain

LGTM! Please clarify the difference in LIGHT settings.

The addition of environment variables for the ethereum service is good. The CHAIN_ID 31337 is appropriate for a local Hardhat network.

Could you please clarify why the LIGHT setting is "true" for ethereum but "false" for arbitrum? This difference might impact performance or functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of LIGHT environment variable
rg --type yaml 'LIGHT:' resources/docker-compose.yml

Length of output: 93

Expand Down Expand Up @@ -116,7 +136,7 @@ services:
ports:
- 3000:3000
restart: unless-stopped

postgres:
image: postgres:alpine
container_name: postgres
Expand All @@ -129,6 +149,18 @@ services:
volumes:
- ./volumes/orderbook_data:/var/lib/postgresql/data

garden-db:
image: postgres:alpine
container_name: garden-db
environment:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: garden
ports:
- 5433:5432
volumes:
- ./volumes/ponder_data:/var/lib/postgresql/data

redis:
image: redis:alpine
container_name: redis
Expand All @@ -137,6 +169,46 @@ services:
volumes:
- ./volumes/cobi_data:/data

solana-validator:
image: ghcr.io/catalogfi/solana-test-validator:latest
container_name : solana-validator
ports:
- 8899:8899
- 8900:8900
restart: unless-stopped

solana-watcher:
image: ghcr.io/catalogfi/solana-watcher:latest
container_name : solana-watcher
depends_on:
- solana-validator
restart: unless-stopped

solana-executor:
image: ghcr.io/catalogfi/solana-executor:latest
container_name: solana-executor
depends_on:
- solana-watcher
restart: unless-stopped

solana-relayer:
image: ghcr.io/catalogfi/solana-relayer:latest
ports:
- 5014:5014
container_name: solana-relayer
command: ["--use-testkey"]
depends_on:
- solana-validator
restart: unless-stopped

virtual-balance:
image: ghcr.io/catalogfi/virtual-balance:latest
container_name: virtual-balance
depends_on:
- ethereum
- electrs
restart: unless-stopped

orderbook:
image: ghcr.io/catalogfi/orderbook:latest
container_name: orderbook
Expand All @@ -161,6 +233,47 @@ services:
extra_hosts:
- "host.docker.internal:host-gateway"

# following services will be part of gardenv2 stack
garden-evm-watcher:
image: ghcr.io/catalogfi/garden-evm-watcher:latest
container_name: garden-evm-watcher
depends_on:
- ethereum
- arbitrum
- chopsticks
- garden-db
restart: unless-stopped
extra_hosts:
- "host.docker.internal:host-gateway"

matcher:
image: ghcr.io/catalogfi/matcher:latest
container_name: matcher
depends_on:
- garden-evm-watcher
restart: unless-stopped
extra_hosts:
- "host.docker.internal:host-gateway"

bit-ponder:
image: ghcr.io/catalogfi/bit-ponder:latest
container_name: bit-ponder
depends_on:
- garden-evm-watcher
- matcher
restart: unless-stopped
extra_hosts:
- "host.docker.internal:host-gateway"

cobiv2:
image: ghcr.io/catalogfi/cobiv2:latest
container_name: cobiv2
depends_on:
- garden-evm-watcher
restart: unless-stopped
extra_hosts:
- "host.docker.internal:host-gateway"

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove trailing space.

There is a trailing space on line 226. This should be removed to adhere to best coding practices and to prevent potential issues in YAML parsing.

Apply this diff to remove the trailing space:

-  
+
📝 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
🧰 Tools
🪛 yamllint

[error] 226-226: trailing spaces

(trailing-spaces)

cosigner:
image: ghcr.io/catalogfi/cosigner:latest
container_name: cosigner
Expand Down Expand Up @@ -190,7 +303,6 @@ services:
- cobi
- cosigner


networks:
default:
name: merry