Skip to content

client/asset/eth: Use gwei in Balance. #1078

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

Merged
chappjc merged 2 commits into from
Jul 20, 2021
Merged

client/asset/eth: Use gwei in Balance. #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
887f147
client/asset/eth: Use gwei in Balance.
JoeGruffins May 7, 2021
e503c61
Ensure no zero division.
JoeGruffins Jul 15, 2021
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
40 changes: 22 additions & 18 deletions client/asset/eth/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func init() {
const (
// BipID is the BIP-0044 asset ID.
BipID = 60
defaultGasFee = 82_000_000_000
defaultGasFeeLimit = 200_000_000_000
defaultGasFee = 82 // gwei
defaultGasFeeLimit = 200 // gwei
)

var (
Expand All @@ -49,7 +49,7 @@ var (
Description: "This is the highest network fee rate you are willing to " +
"pay on swap transactions. If gasfeelimit is lower than a market's " +
"maxfeerate, you will not be able to trade on that market with this " +
"wallet. Units: wei",
"wallet. Units: gwei / gas",
DefaultValue: defaultGasFeeLimit,
},
{
Expand All @@ -62,7 +62,7 @@ var (
// WalletInfo defines some general information about a Ethereum wallet.
WalletInfo = &asset.WalletInfo{
Name: "Ethereum",
Units: "wei",
Units: "gwei",
DefaultConfigPath: defaultAppDir, // Incorrect if changed by user?
ConfigOpts: configOpts,
}
Expand Down Expand Up @@ -219,25 +219,25 @@ func (eth *ExchangeWallet) Connect(ctx context.Context) (*sync.WaitGroup, error)
// OwnsAddress indicates if an address belongs to the wallet.
//
// In Ethereum, an address is an account.
//
// TODO: Consider adding multiple accounts.
func (eth *ExchangeWallet) OwnsAddress(address string) (bool, error) {
return strings.ToLower(eth.acct.Address.String()) == strings.ToLower(address), nil
}

// Balance returns the total available funds in the account.
//
// NOTE: Ethereum balance does not return Immature or Locked values.
// Balance returns the total available funds in the account. The eth node
// returns balances in wei. Those are flored and stored as gwei, or 1e9 wei.
//
// TODO: Ethereum balances can easily go over the max value of a uint64.
// asset.Balance must be changed in a way to accommodate this.
// TODO: Return Immature and Locked values.
func (eth *ExchangeWallet) Balance() (*asset.Balance, error) {
bigbal, err := eth.node.balance(eth.ctx, eth.acct)
bigBal, err := eth.node.balance(eth.ctx, eth.acct)
if err != nil {
return nil, err
}
gwei, err := dexeth.ToGwei(bigBal)
if err != nil {
return nil, err
}
bal := &asset.Balance{
Available: bigbal.Uint64(),
Available: gwei,
// Immature: , How to know?
// Locked: , Not lockable?
}
Expand Down Expand Up @@ -403,14 +403,15 @@ func (eth *ExchangeWallet) sendToAddr(addr common.Address, amt, gasFee *big.Int)
return eth.node.sendTransaction(eth.ctx, tx)
}

// Withdraw withdraws funds to the specified address. Fees are subtracted from
// the value.
// Withdraw withdraws funds to the specified address. Value is gwei.
//
// TODO: Value could be larger than a uint64. Deal with it...
// TODO: Return the asset.Coin.
// TODO: Subtract fees from the value.
func (eth *ExchangeWallet) Withdraw(addr string, value uint64) (asset.Coin, error) {
bigVal := big.NewInt(0).SetUint64(value)
gweiFactorBig := big.NewInt(dexeth.GweiFactor)
_, err := eth.sendToAddr(common.HexToAddress(addr),
big.NewInt(0).SetUint64(value), big.NewInt(0).SetUint64(defaultGasFee))
bigVal.Mul(bigVal, gweiFactorBig), big.NewInt(0).SetUint64(defaultGasFee))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -438,7 +439,10 @@ func (eth *ExchangeWallet) SyncStatus() (bool, float32, error) {
return false, 0, err
}
if sp != nil {
ratio := float32(sp.CurrentBlock) / float32(sp.HighestBlock)
var ratio float32
if sp.HighestBlock != 0 {
ratio = float32(sp.CurrentBlock) / float32(sp.HighestBlock)
}
return false, ratio, nil
}
bh, err := eth.node.bestHeader(eth.ctx)
Expand Down
71 changes: 70 additions & 1 deletion client/asset/eth/eth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type testNode struct {
syncProgErr error
peerInfo []*p2p.PeerInfo
peersErr error
bal *big.Int
balErr error
}

func (n *testNode) connect(ctx context.Context, node *node.Node, addr common.Address) error {
Expand All @@ -59,7 +61,7 @@ func (n *testNode) accounts() []*accounts.Account {
return nil
}
func (n *testNode) balance(ctx context.Context, acct *accounts.Account) (*big.Int, error) {
return nil, nil
return n.bal, n.balErr
}
func (n *testNode) sendTransaction(ctx context.Context, tx map[string]string) (common.Hash, error) {
return common.Hash{}, nil
Expand Down Expand Up @@ -264,3 +266,70 @@ func TestSyncStatus(t *testing.T) {
}
}
}

func TestBalance(t *testing.T) {
maxInt := ^uint64(0)
maxWei := new(big.Int).SetUint64(maxInt)
gweiFactorBig := big.NewInt(dexeth.GweiFactor)
maxWei.Mul(maxWei, gweiFactorBig)
overMaxWei := new(big.Int).Set(maxWei)
overMaxWei.Add(overMaxWei, gweiFactorBig)
tests := []struct {
name string
bal *big.Int
balErr error
wantBal uint64
wantErr bool
}{{
name: "ok zero",
bal: big.NewInt(0),
wantBal: 0,
}, {
name: "ok rounded down",
bal: big.NewInt(dexeth.GweiFactor - 1),
wantBal: 0,
}, {
name: "ok one",
bal: big.NewInt(dexeth.GweiFactor),
wantBal: 1,
}, {
name: "ok max int",
bal: maxWei,
wantBal: maxInt,
}, {
name: "over max int",
bal: overMaxWei,
wantErr: true,
}, {
name: "node balance error",
bal: big.NewInt(0),
balErr: errors.New(""),
wantErr: true,
}}

for _, test := range tests {
ctx, cancel := context.WithCancel(context.Background())
node := &testNode{}
node.bal = test.bal
node.balErr = test.balErr
eth := &ExchangeWallet{
node: node,
ctx: ctx,
log: tLogger,
}
bal, err := eth.Balance()
cancel()
if test.wantErr {
if err == nil {
t.Fatalf("expected error for test %q", test.name)
}
continue
}
if err != nil {
t.Fatalf("unexpected error for test %q: %v", test.name, err)
}
if bal.Available != test.wantBal {
t.Fatalf("want available balance %v got %v for test %q", test.wantBal, bal.Available, test.name)
}
}
}
7 changes: 4 additions & 3 deletions dex/testing/eth/harness.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ cat > "${NODES_ROOT}/genesis.json" <<EOF
"byzantiumBlock": 0,
"constantinopleBlock": 0,
"petersburgBlock": 0,
"istanbulBlock": 0,
"muirGlacierBlock": 0,
"berlinBlock": 0,
Comment on lines +81 to +83
Copy link
Member Author

@JoeGruffins JoeGruffins Jul 15, 2021

Choose a reason for hiding this comment

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

Should put us at the same.. fork as mainnet.

"clique": {
"period": 1,
"epoch": 30000
Expand Down Expand Up @@ -183,9 +186,7 @@ echo "Sending 5000 eth to delta and gamma."

# Our transactions will PROBABLY appear in one of these blocks.
echo "Mining some blocks"
"${NODES_ROOT}/harness-ctl/mine-beta" "1"
"${NODES_ROOT}/harness-ctl/mine-alpha" "1"
"${NODES_ROOT}/harness-ctl/mine-beta" "5"
"${NODES_ROOT}/harness-ctl/mine-beta" "15"
Comment on lines -186 to +189
Copy link
Member Author

@JoeGruffins JoeGruffins Jul 15, 2021

Choose a reason for hiding this comment

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

I had a few times where it looked like mining on alpha made transaction inclusion speed worse. Will attempt to solve the problem in a separate pr.

Copy link
Member

@chappjc chappjc Jul 15, 2021

Choose a reason for hiding this comment

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

Dang what a drag. Linking back to the original discussion and karalabe's comment in a different thread.

Copy link
Member

@chappjc chappjc Jul 15, 2021

Choose a reason for hiding this comment

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

Another related idea was to have the non-miners (clients) connected to each other or at least another non-miner (or not the primary miner): ethereum/go-ethereum#2822 (comment)

ivooz: The problem goes away when all the non-mining nodes are connected to each other, previously they only knew about miner nodes.

karalabe: ... your first topology were that all nodes were connected only to the miners. This can be problematic as the miners are heavily loaded churning out blocks, so if everyone is bugging them to distribute the data too, I can imagine that they are simply overwhelmed (you essentially DDOS the miners).

I'd recommend enabling discovery (with a custom bootnode, maybe one of the miners, should be fine) so nodes can properly connect to each other and form a decent overlay network to propagate the data.

Copy link
Member

@chappjc chappjc Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Oh and about the first idea of starting 2 miners, one guy said he "updated the file "static-nodes.json" accordingly". Know what we should do there? Specify enodes? Sounds like an alt was karalabe's other suggestion of enabling discovery with a custom bootnode.

Copy link
Member Author

@JoeGruffins JoeGruffins Jul 16, 2021

Choose a reason for hiding this comment

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

Will look into this directly after I get #1019 up to speed.

Copy link
Member Author

@JoeGruffins JoeGruffins Jul 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, still scratching my head. These convos are from 2016, so maybe something different. Also, putting sleeps in the wrong places in the harness can just make everything fail. Which seems wierd.

Copy link
Member Author

@JoeGruffins JoeGruffins Jul 19, 2021

Choose a reason for hiding this comment

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

Pretty sure I found and fixed the problem in #1130 Calling miner.start() miner.stop() in quick succession seems to mess with the ability of txs to make it into blocks.


# Initial sync for light nodes takes quite a while. Wait for them to show
# blocks on the network.
Expand Down
19 changes: 17 additions & 2 deletions server/asset/eth/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,34 @@ package eth
import (
"encoding/binary"
"fmt"
"math/big"

"github.com/ethereum/go-ethereum/common"
)

// coinIdSize = flags (2) + smart contract address where funds are locked (20) + secret
// hash map key (32)
const (
// coinIdSize = flags (2) + smart contract address where funds are
// locked (20) + secret hash map key (32)
coinIDSize = 54
// MaxBlockInterval is the number of seconds since the last header came
// in over which we consider the chain to be out of sync.
MaxBlockInterval = 180
// GweiFactor is the amount of wei in one gwei. Eth balances are floored
// as gwei, or 1e9 wei. This is used in factoring.
GweiFactor = 1e9
)

// ToGwei converts a *big.Int in wei (1e18 unit) to gwei (1e9 unit) as a uint64.
// Errors if the amount of gwei is too big to fit fully into a uint64.
func ToGwei(wei *big.Int) (uint64, error) {
gweiFactorBig := big.NewInt(GweiFactor)
wei.Div(wei, gweiFactorBig)
if !wei.IsUint64() {
return 0, fmt.Errorf("suggest gas price %v gwei is too big for a uint64", wei)
}
return wei.Uint64(), nil
}

// DecodeCoinID decodes the coin ID into flags, a contract address, and secret hash.
func DecodeCoinID(coinID []byte) (uint16, common.Address, []byte, error) {
if len(coinID) != coinIDSize {
Expand Down
14 changes: 1 addition & 13 deletions server/asset/eth/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,11 @@ const (
// The blockPollInterval is the delay between calls to bestBlockHash to
// check for new blocks.
blockPollInterval = time.Second
gweiFactor = 1e9
)

var (
zeroHash = common.Hash{}
notImplementedErr = errors.New("not implemented")
gweiFactorBig = big.NewInt(gweiFactor)
_ asset.Driver = (*Driver)(nil)
)

Expand Down Expand Up @@ -71,16 +69,6 @@ type ethFetcher interface {
peers(ctx context.Context) ([]*p2p.PeerInfo, error)
}

// toGwei converts a *big.Int in wei (1e18 unit) to gwei (1e9 unit) as a uint64.
// Errors if the amount of gwei is too big to fit fully into a uint64.
func toGwei(wei *big.Int) (uint64, error) {
wei.Div(wei, gweiFactorBig)
if !wei.IsUint64() {
return 0, fmt.Errorf("suggest gas price %v gwei is too big for a uint64", wei)
}
return wei.Uint64(), nil
}

// Backend is an asset backend for Ethereum. It has methods for fetching output
// information and subscribing to block updates. It maintains a cache of block
// data for quick lookups. Backend implements asset.Backend, so provides
Expand Down Expand Up @@ -190,7 +178,7 @@ func (eth *Backend) FeeRate() (uint64, error) {
if err != nil {
return 0, err
}
return toGwei(bigGP)
return ToGwei(bigGP)
}

// BlockChannel creates and returns a new channel on which to receive block
Expand Down
5 changes: 3 additions & 2 deletions server/asset/eth/eth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ func TestRun(t *testing.T) {
func TestFeeRate(t *testing.T) {
maxInt := ^uint64(0)
maxWei := new(big.Int).SetUint64(maxInt)
gweiFactorBig := big.NewInt(GweiFactor)
maxWei.Mul(maxWei, gweiFactorBig)
overMaxWei := new(big.Int).Set(maxWei)
overMaxWei.Add(overMaxWei, gweiFactorBig)
Expand All @@ -287,11 +288,11 @@ func TestFeeRate(t *testing.T) {
wantFee: 0,
}, {
name: "ok rounded down",
gas: big.NewInt(gweiFactor - 1),
gas: big.NewInt(GweiFactor - 1),
wantFee: 0,
}, {
name: "ok one",
gas: big.NewInt(gweiFactor),
gas: big.NewInt(GweiFactor),
wantFee: 1,
}, {
name: "ok max int",
Expand Down