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

multi: Add internal ethereum clients. #1005

Merged
merged 6 commits into from Jul 15, 2021

Conversation

JoeGruffins
Copy link
Member

@JoeGruffins JoeGruffins commented Mar 8, 2021

multi: Add internal ethereum clients.

Adds an eth light node and wallet to client/asset. This is a bare bones
implementation and most ExchangeWallet methods do not work. Mainnet use
is disabled.

closes #939

@JoeGruffins JoeGruffins force-pushed the dexethclientasset branch 2 times, most recently from 42a53ac to 708ab8d Compare March 9, 2021 07:52
@JoeGruffins JoeGruffins force-pushed the dexethclientasset branch 2 times, most recently from 104be68 to 675d177 Compare March 12, 2021 09:04
Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

Coming along nicely, @JoeGruffins.

client/asset/eth/eth.go Outdated Show resolved Hide resolved
@JoeGruffins JoeGruffins force-pushed the dexethclientasset branch 5 times, most recently from b44db94 to 4600635 Compare March 17, 2021 10:53
@chappjc chappjc added this to the 0.3 milestone Apr 7, 2021
Copy link
Member

@vctt94 vctt94 left a comment

Choose a reason for hiding this comment

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

Good job on this PR joe!

So I am testing using ethereum's goerli network, and when first starting I get

failed to start asset "eth": unable to dial rpc: dial unix /home/vctt/.ethereum/geth/geth.ipc: connect: no such file or directory

As my geth.ipc is in the goerli's directory.

But if I add an ethereum.conf I get the following:

failed to start asset "eth": unable to dial rpc: dial unix /home/vctt/.ethereum/ethereum.conf: connect: connection refused

If I change the default geth ipc path, I get the following, when syncing:

2021-04-21 14:29:05.496 [INF] AUTH: Allowing 6 settling + taker order lots per market for new users.
2021-04-21 14:29:05.496 [INF] AUTH: Allowing up to 375 settling + taker order lots per market for established users.
2021-04-21 14:29:05.506 [INF] MAIN: The DEX is running. Hit CTRL+C to quit...
2021-04-21 14:29:05.506 [INF] COMM: RPC server listening on 127.0.0.1:7232
2021-04-21 14:29:06.000 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42
2021-04-21 14:29:10.001 [INF] MKT: Market dcr_btc now accepting orders, epoch 161901535:10000
2021-04-21 14:29:12.001 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42

Not sure if this is because of my goeli network sync, but they are up to date with the ether explorer

@JoeGruffins
Copy link
Member Author

@vctt94 yeah the sync checking logic could definitely be wrong.

@JoeGruffins JoeGruffins force-pushed the dexethclientasset branch 4 times, most recently from 36e21c9 to ea51165 Compare May 5, 2021 07:21
@JoeGruffins JoeGruffins changed the title client/assets: Add ethereum. multi: Add internal ethereum clients. May 5, 2021
@JoeGruffins
Copy link
Member Author

JoeGruffins commented May 5, 2021

Not perfect but hopefully good enough. Disabled on mainnet, and most of the functions don't work anyway so I doubt anyone can hurt themselves with this.

Sync status is probably not right and needs more study.

Another problem is that it takes ages for transactions to be mined. I'm not sure at all if that's normal or maybe something as simple as a mistake with fee values I am making.

@JoeGruffins JoeGruffins marked this pull request as ready for review May 5, 2021 07:26
@JoeGruffins
Copy link
Member Author

JoeGruffins commented May 7, 2021

@vctt94 Is this the server starting?

It doesn't actually accept a config atm. It takes the path to the ipc, since nothing else is needed to connect to the node.
"configPath": "${TEST_ROOT}/eth/alpha/node/geth.ipc"
Perhaps this is confusing?

Attempting to address confusion with #1080

Copy link
Member

@vctt94 vctt94 left a comment

Choose a reason for hiding this comment

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

dcrdex server is starting but it looks like still not syncing. From logs:

vctt@DESKTOP-NMIS6QR:~/projects/decred/dcrdex/server/cmd/dcrdex$ go build && ./dcrdex --testnet
2021-05-09 14:23:38.076 [INF] MAIN: App data folder: /home/vctt/.dcrdex
2021-05-09 14:23:38.076 [INF] MAIN: Data folder:     /home/vctt/.dcrdex/data/testnet
2021-05-09 14:23:38.076 [INF] MAIN: Log folder:      /home/vctt/.dcrdex/logs/testnet
2021-05-09 14:23:38.076 [INF] MAIN: Config file:     /home/vctt/.dcrdex/dcrdex.conf
2021-05-09 14:23:38.076 [INF] MAIN: Logging with UTC time stamps. Current local time is 11:23:38 -03
2021-05-09 14:23:38.076 [INF] MAIN: dcrdex version 0.2.0-pre+dev (Go version go1.15.6)
2021-05-09 14:23:38.076 [INF] MAIN: dcrdex starting for network: testnet
2021-05-09 14:23:38.076 [INF] MAIN: swap locktimes config: maker 20h0m0s, taker 8h0m0s
2021-05-09 14:23:38.076 [DBG] MAIN: -------------------- BEGIN parsed markets.json --------------------
2021-05-09 14:23:38.076 [DBG] MAIN: MARKETS
2021-05-09 14:23:38.076 [DBG] MAIN:                   Base         Quote   EpochDur
2021-05-09 14:23:38.076 [DBG] MAIN: Market 0:  DCR_testnet   BTC_testnet     10000 ms
2021-05-09 14:23:38.076 [DBG] MAIN: Market 1:  DCR_testnet   ETH_testnet      6000 ms
2021-05-09 14:23:38.076 [DBG] MAIN: 
2021-05-09 14:23:38.076 [DBG] MAIN: ASSETS
2021-05-09 14:23:38.076 [DBG] MAIN:                   LotSize     RateStep   MaxFeeRate   Network
2021-05-09 14:23:38.076 [DBG] MAIN: BTC_mainnet        100000       100000          100   mainnet
2021-05-09 14:23:38.076 [DBG] MAIN: BTC_testnet        100000       100000          100   testnet
2021-05-09 14:23:38.076 [DBG] MAIN: LTC_mainnet       1000000      1000000           20   mainnet
2021-05-09 14:23:38.076 [DBG] MAIN: ETH_testnet     100000000    100000000           10   testnet
2021-05-09 14:23:38.076 [DBG] MAIN: DCR_mainnet     100000000    100000000           10   mainnet
2021-05-09 14:23:38.076 [DBG] MAIN: DCR_testnet     100000000    100000000           10   testnet
2021-05-09 14:23:38.076 [DBG] MAIN: --------------------- END parsed markets.json ---------------------
2021-05-09 14:23:38.076 [INF] MAIN: Found 3 assets, loaded 2 markets, for network TESTNET
Signing key password: 
2021-05-09 14:23:38.871 [INF] MAIN: Loading DEX signing key from /home/vctt/.dcrdex/sigkey...
2021-05-09 14:23:38.908 [INF] DEX: Starting asset backend "btc"...
2021-05-09 14:23:38.913 [INF] DEX: Starting asset backend "dcr"...
2021-05-09 14:23:38.921 [INF] ASSET[dcr]: Connected to dcrd (JSON-RPC API v6.2.0) on TestNet3
2021-05-09 14:23:38.922 [INF] DEX: Starting asset backend "eth"...
2021-05-09 14:23:38.944 [WRN] DEX: Error priming fee cache for eth: not implemented
2021-05-09 14:23:38.966 [INF] DB: Switching PostgreSQL time zone to UTC for this session.
2021-05-09 14:23:38.968 [INF] DB: PostgreSQL 13.2 (Debian 13.2-1.pgdg100+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
2021-05-09 14:23:38.974 [WRN] DB: PERFORMANCE ISSUE! The synchronous_commit setting is "on". Changing it to "off".
2021-05-09 14:23:38.990 [INF] DB: DCRDEX database ready at version 2
2021-05-09 14:23:38.990 [INF] DB: Configuring 2 markets tables: [dcr_btc dcr_eth]
2021-05-09 14:23:39.008 [INF] DEX: Cancellation rate threshold 0.950000, new user grace period 19 cancels
2021-05-09 14:23:39.008 [INF] DEX: MIA user order unbook timeout 12m0s
2021-05-09 14:23:39.008 [INF] DEX: Ban score threshold is 20
2021-05-09 14:23:39.010 [INF] SWAP: Loaded swap data for 0 active swaps.
2021-05-09 14:23:39.011 [INF] MKT: Allowing 4294967295 lots on the book per user.
2021-05-09 14:23:39.011 [INF] MKT: Loaded 0 stored book orders.
2021-05-09 14:23:39.011 [DBG] MKT: Locking 0 base asset (42) coins.
2021-05-09 14:23:39.011 [DBG] MKT: Locking 0 quote asset (0) coins.
2021-05-09 14:23:39.013 [INF] MKT: Tracking 0 orders with 0 active matches.
2021-05-09 14:23:39.013 [INF] DEX: Preparing historical market data API for market dcr_btc...
2021-05-09 14:23:39.160 [DBG] DB: select epoch candles in: 147.410469ms
2021-05-09 14:23:39.161 [INF] MKT: Allowing 4294967295 lots on the book per user.
2021-05-09 14:23:39.161 [INF] MKT: Loaded 0 stored book orders.
2021-05-09 14:23:39.161 [DBG] MKT: Locking 0 base asset (42) coins.
2021-05-09 14:23:39.161 [DBG] MKT: Locking 0 quote asset (60) coins.
2021-05-09 14:23:39.162 [INF] MKT: Tracking 0 orders with 0 active matches.
2021-05-09 14:23:39.162 [INF] DEX: Preparing historical market data API for market dcr_eth...
2021-05-09 14:23:39.163 [DBG] DB: select epoch candles in: 1.009183ms
2021-05-09 14:23:39.163 [DBG] AUTH: Expecting 0 users with booked orders to connect within 12m0s
2021-05-09 14:23:39.163 [INF] AUTH: Allowing 6 settling + taker order lots per market for new users.
2021-05-09 14:23:39.163 [INF] AUTH: Allowing up to 375 settling + taker order lots per market for established users.
2021-05-09 14:23:39.163 [DBG] SWAP: Swapper started with 12m0s broadcast timeout.
2021-05-09 14:23:39.169 [INF] MAIN: The DEX is running. Hit CTRL+C to quit...
2021-05-09 14:23:39.169 [INF] COMM: RPC server listening on 127.0.0.1:7232
2021-05-09 14:23:40.010 [INF] MKT: Market dcr_btc now accepting orders, epoch 162057022:10000
2021-05-09 14:23:42.001 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42
2021-05-09 14:23:48.000 [ERR] DEX: Error retrieving fee rate for eth: not implemented
2021-05-09 14:23:48.001 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42
2021-05-09 14:23:54.000 [ERR] DEX: Error retrieving fee rate for eth: not implemented
2021-05-09 14:23:54.001 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42
2021-05-09 14:24:00.000 [ERR] DEX: Error retrieving fee rate for eth: not implemented
2021-05-09 14:24:00.001 [ERR] MKT: Not starting dcr_eth market because of ChainsSynced error: Error checking sync status for 42

The ipc config now works fine. I think it is ok having the geth.ipc on the config path, as we don't need any other param on the config right now, but do you think we might have other config params in the future?

@JoeGruffins
Copy link
Member Author

JoeGruffins commented May 10, 2021

but do you think we might have other config params in the future?

I don't know...

Error checking sync status for 42

42 is dcr tho. I will try try this out on testnet.

edit:
Attempting to fix with #1082

@JoeGruffins
Copy link
Member Author

I feel like the rpcclient methods have too much logic. I think I will move some of it.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

First pass. Looking good.

client/asset/eth/eth.go Show resolved Hide resolved
client/asset/eth/eth.go Outdated Show resolved Hide resolved
client/asset/eth/eth.go Outdated Show resolved Hide resolved
type ethFetcher interface {
accounts() []*accounts.Account
addPeer(ctx context.Context, peer string) error
balance(ctx context.Context, acct *accounts.Account) (*big.Int, error)
Copy link
Member

Choose a reason for hiding this comment

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

How about *big.Int -> uint64 and convert to wei internally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it be ok to revisit this in #1078 ? I would really like to keep as much logic as possible out of rpcclient.go to allow more thorough testing. I would like the rpc calls to return as close to verbatim as possible. Coverting to uint64 comes with a check that the big Int isn't larger than a uint64.

}
bal := &asset.Balance{
Available: bigbal.Uint64(),
// Immature: , How to know?
Copy link
Member

Choose a reason for hiding this comment

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

func (ec *Client) BalanceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (*big.Int, error)

Note the blockNumber parameter. I think we could use that to get "immature" balance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like we should be able to do that. Can I put this in a new pr along with the following Locked? What's a good number of confs for eth? If we wanted 6 confs, then I think the mature balance would be the lowest balance from the last six blocks. Subtract from total to get immature.

Copy link
Member

Choose a reason for hiding this comment

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

This is really confusing to me because I thought you had to have an archival node to access blockchain data at arbitrary heights.

Copy link
Member Author

Choose a reason for hiding this comment

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

While I'd rather not delve into how this all works... Maybe no choice? Could be a deep rabbit hole. I'm guessing it queries other nodes for this info. A fast or snap node should have the last some trie states(?) in memory. I think I experimented on our simnet and the light nodes didn't have a problem telling me balances of past blocks. Will do some more testing though.

Unless you had a different idea about how to get the "mature" balance, as we choose to define it.

Copy link
Member Author

Choose a reason for hiding this comment

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

On mainnet with a node using light mode:

> eth.getBalance("0x7C4240a8e7F60f52ca2B3b0E80B539bE8eFE42f8", eth.blockNumber)
2973712609626249
> eth.getBalance("0x7C4240a8e7F60f52ca2B3b0E80B539bE8eFE42f8", eth.blockNumber-127)
2973712609626249
> eth.getBalance("0x7C4240a8e7F60f52ca2B3b0E80B539bE8eFE42f8", eth.blockNumber-128)
2973712609626249
> eth.getBalance("0x7C4240a8e7F60f52ca2B3b0E80B539bE8eFE42f8", eth.blockNumber-129)
Error: getDeleteStateObject (7c4240a8e7f60f52ca2b3b0e80b539be8efe42f8) error: no suitable peers available
        at web3.js:6347:37(47)
        at web3.js:5081:62(37)
        at <eval>:1:15(8)

> 

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm assuming it shouldn't be a problem for the last ~10 blocks...

Copy link
Member

Choose a reason for hiding this comment

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

https://ethereum.stackexchange.com/a/55424

Looks like a risky assumption.

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking anything "immature" will not come from geth, but rather our swap contract tracking or other txn monitoring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds fine to me....

client/asset/eth/eth.go Outdated Show resolved Hide resolved
client/asset/eth/rpcclient.go Outdated Show resolved Hide resolved
}

// transactionReceipt uses a raw request to retrieve a transaction's receipt.
func (c *rpcclient) transactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) {
Copy link
Member

Choose a reason for hiding this comment

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

transactionReceipt, pendingTransactions, addPeer, blockNumber, and importAccount are only used in rpcclient_harness_test.go, and are all just wrappers around CallContext. Rather than having these methods as part of the ethFetcher interface, requiring implementation in test stubs, consider moving these methods to fuctions in rpcclient_harness_test.go itself with a helper function for the CallContext calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it ok to leave for now? I am not sure which may be used later in other methods. I would like to do this once the dust clears.

Comment on lines +196 to +205
# Transactions can take an eternity to be mined...
# TODO: Determine why this is.
Copy link
Member

Choose a reason for hiding this comment

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

Ugh. Looks like no simple solution. https://ethereum.stackexchange.com/questions/2539/how-do-i-decrease-the-difficulty-on-a-private-testnet

Would it be better to prevent the difficulty increase by delaying mining by 15 seconds between each block?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure this is the same problem. Mining is fine because we use oracles to create blocks, there isn't any actual mining. We are using clique in the test harness. It could be related though. Transactions just seem to get put into blocks randomly.

Copy link
Member

Choose a reason for hiding this comment

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

Seems very likely a quirk/bug related to how geth behaves with only one node mining: ethereum/go-ethereum#2769 (comment)
Sounds like a solution is to have another node mine at least one block. So way up above at mine-alpha" "15", we could try a: mine-alpha 1 -> mine-beta 1 -> then go ahead and mine as needed again on alpha.

Copy link
Member Author

Choose a reason for hiding this comment

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

Trying that, it's really more noticeable in the test in #1019

server/asset/eth/common.go Show resolved Hide resolved
@JoeGruffins JoeGruffins force-pushed the dexethclientasset branch 2 times, most recently from 5f175a2 to d07c22f Compare June 9, 2021 02:52
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

@JoeGruffins - boldly going where no decred developer has gone before. Thanks for this work. Some small requests aside, we can get this merged soon, and ETH progress can really start building on top of this.

dex/testing/eth/harness.sh Outdated Show resolved Hide resolved
@@ -86,14 +86,20 @@ cat > "${NODES_ROOT}/genesis.json" <<EOF
},
"4f8ef3892b65ed7fc356ff473a2ef2ae5ec27a06": {
"balance": "11000000000000000000000"
},
Copy link
Member

Choose a reason for hiding this comment

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

At the top of harness.sh, pls put a comment summarizing the different full/light nodes and their intended use (server vs. client)

Copy link
Member Author

Choose a reason for hiding this comment

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

# tmux script that sets up an eth simnet harness. It sets up four separate nodes.
# alpha and beta nodes are synced in snap mode. They emulate nodes used by the
# dcrdex server. Either has the authority to mine blocks. They start with
# pre-allocated funds. gamma and delta are synced in light mode and emulate
# nodes used by dexc. They are sent some funds after being created. The harness
# waits for all nodes to sync before allowing tmux input.

Comment on lines +196 to +205
# Transactions can take an eternity to be mined...
# TODO: Determine why this is.
Copy link
Member

Choose a reason for hiding this comment

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

Seems very likely a quirk/bug related to how geth behaves with only one node mining: ethereum/go-ethereum#2769 (comment)
Sounds like a solution is to have another node mine at least one block. So way up above at mine-alpha" "15", we could try a: mine-alpha 1 -> mine-beta 1 -> then go ahead and mine as needed again on alpha.

go.mod Outdated Show resolved Hide resolved
client/asset/eth/config.go Outdated Show resolved Hide resolved
client/asset/eth/rpcclient.go Outdated Show resolved Hide resolved
// the value.
//
// TODO: Value could be larger than a uint64. Deal with it...
// TODO: Return the asset.Coin.
Copy link
Member

@chappjc chappjc Jul 13, 2021

Choose a reason for hiding this comment

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

Assuming the return is a TODO because coin ID needs adaptation to support general txids (no contract business)? i.e. "withdrawals" in #1021 So basically a matter of recognizing new flags and having a variable coin id byte length.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is not set in stone what the return should look like yet, with flags. I'm thinking just 0 for normal tx, 1 for contract plus secret hash, I think we have an issue about it.

client/asset/eth/eth.go Outdated Show resolved Hide resolved
client/asset/eth/node.go Outdated Show resolved Hide resolved
client/asset/eth/node.go Outdated Show resolved Hide resolved
Adds an eth light node and wallet to client/asset. This is a bare bones
implementation and most ExchangeWallet methods do not work. Mainnet use
is disabled.
if len(peers) < requiredNPeers {
return false, 0, nil
if sp != nil {
ratio := float32(sp.CurrentBlock) / float32(sp.HighestBlock)
Copy link
Member Author

Choose a reason for hiding this comment

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

wonder if sp.HighestBlock can be 0...

@@ -24,19 +24,18 @@ require (
github.com/decred/dcrd/wire v1.4.0
github.com/decred/go-socks v1.1.0
github.com/decred/slog v1.1.0
github.com/ethereum/go-ethereum v1.9.25
github.com/ethereum/go-ethereum v1.10.4
Copy link
Member

Choose a reason for hiding this comment

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

Aaaand they released 1.10.5 last night 😆
https://github.com/ethereum/go-ethereum/releases/tag/v1.10.5

Copy link
Member Author

Choose a reason for hiding this comment

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

updating...

Copy link
Member Author

Choose a reason for hiding this comment

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

And wouldn't you know it broke something.

Copy link
Member

Choose a reason for hiding this comment

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

Screw it. Can be in another PR. I don't want to hold you up more

@chappjc chappjc merged commit 16da72f into decred:master Jul 15, 2021
@chappjc chappjc added the ETH label Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

backends: Add Ethereum.
5 participants