loadbot: Allow use with all markets. #1656
Conversation
|
Let's update loadbot's go.mod module decred.org/dcrdex/dex/testing/loadbot
go 1.17
// go mod tidy while commented:
// replace decred.org/dcrdex => ../../../
require (
decred.org/dcrdex v0.0.0-20220609033337-f9b5b4ccdfac
github.com/Shopify/toxiproxy/v2 v2.4.0
)
Just need to update the toxiproxy import path: diff --git a/dex/testing/loadbot/loadbot.go b/dex/testing/loadbot/loadbot.go
index 2032fd2b..7f65966c 100644
--- a/dex/testing/loadbot/loadbot.go
+++ b/dex/testing/loadbot/loadbot.go
@@ -32,7 +32,7 @@ import (
"decred.org/dcrdex/dex/config"
dexbtc "decred.org/dcrdex/dex/networks/btc"
dexsrv "decred.org/dcrdex/server/dex"
- toxiproxy "github.com/Shopify/toxiproxy/client"
+ toxiproxy "github.com/Shopify/toxiproxy/v2/client"
) |
JoeGruffins
force-pushed
the
loadbototherassets
branch
4 times, most recently
from
557e9a9
to
b8a8778
Compare
|
Working mostly for all coins. Some amounts still need to be adjusted, but I'm unsure of what's best. I think they can be adjusted as others see fit. For example, the heavy suite needs lots of funds that some harnesses do not have enough of. The Zec compound tests fails with: All compound suites involving dcr seem to fail with this eventually: Other than that I think this pr solves the issue. |
Hmm, I guess that Could this relate to restrictions in spending coinbase outputs? zcash/zcash#1431 |
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
b8a8778
to
2b68ed6
Compare
|
Just rebased. |
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
2b68ed6
to
67190d7
Compare
It looks like the issue is fixed in that pr? Maybe just trying to send to many coins too fast? I don't see this with just the pingpong suite. I did see this with the pingpong however: pingpong is mostly fine it seems. |
| @@ -219,12 +219,6 @@ tmux kill-session | |||
| EOF | |||
| chmod +x "${NODES_ROOT}/harness-ctl/quit" | |||
|
|
|||
| cat > "${NODES_ROOT}/harness-ctl/new-wallet" <<EOF | |||
There was a problem hiding this comment.
Everyone OK with losing the new-wallet shell script? Personally I use the general node wrapper directly like ./alpha createwallet, so it's fine with me.
There was a problem hiding this comment.
Looks like that's what @JoeGruffins is doing now in loadbot, so we can lose it, I think.
There was a problem hiding this comment.
Right, I meant for tooling around in the harness control tmux.
There was a problem hiding this comment.
It was only one rpc call, so I didn't think it had a good purpose for the coins it existed in. The new start-wallet script has a lot of set-up so I sorta moved this there for places where I thought it was needed.
There was a problem hiding this comment.
Although, the btc create ended up really long because I needed to put in a lot of args to turn descriptor wallets off. Maybe I should put it back for more visibility in the harness whenever they change and become use-able. I see some prs about them.
There was a problem hiding this comment.
Meh I think we can live without new-wallet.
Descriptor wallets are going to be fine really soon anyway
There was a problem hiding this comment.
I added it back for btc, bch, and litecoin and think it's useful... Can remove again tho.
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
67190d7
to
04faf33
Compare
|
Added back new-wallet for bitcoin assets and added info to the markets error. https://github.com/decred/dcrdex/compare/67190d7e42b1b1ba647c28f3894f2db53eef13e9..04faf33b6b728330d8afc15ee9bbf28402c2b73b |
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
04faf33
to
493de78
Compare
|
Addressing marton's doc comments: https://github.com/decred/dcrdex/compare/04faf33b6b728330d8afc15ee9bbf28402c2b73b..493de785759483d6f9a481500af2c0ed26524d68 |
dex/testing/loadbot/loadbot.go
Outdated
| func nextAddr() (string, string) { | ||
| port := strconv.Itoa(int(atomic.AddUint32(&portCounter, 1) + startPort)) | ||
| port := strconv.Itoa(int(atomic.AddUint32(&atomicPort, 1))) | ||
| return "127.0.0.1:" + port, port | ||
| } |
There was a problem hiding this comment.
Since we don't care what these ports are, something I've been playing with to prevent any chance of running into an in-use port is
func findOpenAddrs(n int) ([]net.Addr, error) {
addrs := make([]net.Addr, 0, n)
for i := 0; i < n; i++ {
addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
if err != nil {
return nil, err
}
l, err := net.ListenTCP("tcp", addr)
if err != nil {
return nil, err
}
defer l.Close()
addrs = append(addrs, l.Addr())
}
return addrs, nil
}This might actually become important if we ever want to run > 1 instance of loadbot simultaneously.
There was a problem hiding this comment.
Seems useful to do. There's still a race there after l.Close that the system will give it away, but it's way better than nothing.
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
493de78
to
a68155e
Compare
|
Just rebased. |
|
Saw this for the first time on dcr_doge: |
|
And: |
|
And: Maybe need to set the client locktimes correctly for this one. They are using the default while server is the 30s/1m Oh that's right, I should be building with the tags... |
|
Still seeing that last one even after building properly. Will make some issues I guess. |
JoeGruffins
force-pushed
the
loadbototherassets
branch
from
a68155e
to
a2ad03d
Compare
|
Regarding revoked matches in loadbot, while testing, I had wondered if it was even a good idea to use it with 30s/1m lock times rather than much higher. I need to check how the new block mining is triggered from loadbot, and if it's possible matches will occasionally just be revoked if not settled fast enough. (Given the recent proactive revoke changes to Swapper.) |
|
No idea about the DOGE signing error (or is it spending a spent output?). Definitely make that an issue. The order validation might just be a case of inadequate error detail - probably an order that was given a bad rate or quantity by loadbot. |
|
The mining is rather random. I guess they overlap too. I don't plan to change this pr any more unless there are requests to do so. Unless you have an idea for what the locktimes would be best, can change the comment. So far no errors at all seen on the eth market! (knock on wood) |
|
Nice! I've run this PR a bunch and have little trouble aside from |
|
Hmm, not sure how this can happen. Can doge overflow? : I should have had debugging on Oh What. |
More likely underflow. Kinda wish we were using signed int64 for amounts more broadly. |
closes #1565