-
Notifications
You must be signed in to change notification settings - Fork 102
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
Conversation
42a53ac
to
708ab8d
Compare
104be68
to
675d177
Compare
There was a problem hiding this 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.
b44db94
to
4600635
Compare
There was a problem hiding this 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
@vctt94 yeah the sync checking logic could definitely be wrong. |
36e21c9
to
ea51165
Compare
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. |
There was a problem hiding this 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?
I don't know...
42 is dcr tho. I will try try this out on testnet. edit: |
I feel like the rpcclient methods have too much logic. I think I will move some of it. |
c1ce7e0
to
66e9e16
Compare
There was a problem hiding this 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.
type ethFetcher interface { | ||
accounts() []*accounts.Account | ||
addPeer(ctx context.Context, peer string) error | ||
balance(ctx context.Context, acct *accounts.Account) (*big.Int, error) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
>
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me....
} | ||
|
||
// transactionReceipt uses a raw request to retrieve a transaction's receipt. | ||
func (c *rpcclient) transactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
# Transactions can take an eternity to be mined... | ||
# TODO: Determine why this is. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
5f175a2
to
d07c22f
Compare
There was a problem hiding this 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.
@@ -86,14 +86,20 @@ cat > "${NODES_ROOT}/genesis.json" <<EOF | |||
}, | |||
"4f8ef3892b65ed7fc356ff473a2ef2ae5ec27a06": { | |||
"balance": "11000000000000000000000" | |||
}, |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
# Transactions can take an eternity to be mined... | ||
# TODO: Determine why this is. |
There was a problem hiding this comment.
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.
// the value. | ||
// | ||
// TODO: Value could be larger than a uint64. Deal with it... | ||
// TODO: Return the asset.Coin. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
197aaca
to
cc2b336
Compare
if len(peers) < requiredNPeers { | ||
return false, 0, nil | ||
if sp != nil { | ||
ratio := float32(sp.CurrentBlock) / float32(sp.HighestBlock) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updating...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
closes #939