client/{webserver,app}: Update Market Making UI #2491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoeGruffins
reviewed
Sep 4, 2023
buck54321
requested changes
Sep 7, 2023
buck54321
requested changes
Sep 11, 2023
client/mm/mm.go
Outdated
Comment on lines
162
to
177
| syncedOracleMtx sync.RWMutex | ||
| syncedOracle *priceOracle | ||
|
|
||
| unsyncedOracle *priceOracle |
There was a problem hiding this comment.
Please doc the difference between syncedOracle and unsyncedOracle.
client/mm/mm.go
Outdated
Comment on lines
285
to
319
| m.syncedOracleMtx.RUnlock() | ||
| if err != nil && !errors.Is(err, errUnsyncedMarket) { | ||
| m.log.Errorf("failed to get oracle info for market %d-%d: %v", base, quote, err) | ||
| } | ||
| if err == nil { | ||
| return &MarketReport{ | ||
| Price: price, | ||
| Oracles: oracles, | ||
| BaseFiatRate: baseFiatRate, | ||
| QuoteFiatRate: quoteFiatRate, | ||
| }, nil | ||
| } | ||
| } | ||
| m.syncedOracleMtx.RUnlock() |
There was a problem hiding this comment.
Won't this double RUnlock if err != nil?
client/mm/mm.go
Outdated
Comment on lines
278
to
280
| user := m.core.User() | ||
| baseFiatRate := user.FiatRates[base] | ||
| quoteFiatRate := user.FiatRates[quote] |
There was a problem hiding this comment.
User is a bit much. Maybe buck54321/dcrdex@42a3766...buck54321:dcrdex:core-fiat-rates
client/webserver/api.go
Outdated
Comment on lines
1675
to
1690
| func (s *WebServer) getMarketMakingConfig() ([]*mm.BotConfig, error) { | ||
| cfg := []*mm.BotConfig{} | ||
| if s.mmCfgPath != "" { | ||
| data, err := ioutil.ReadFile(s.mmCfgPath) | ||
| if err == nil { | ||
| err = json.Unmarshal(data, &cfg) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error unmarshalling market making config: %v", err) | ||
| } | ||
| } else if !os.IsNotExist(err) { | ||
| return nil, fmt.Errorf("error reading file: %v", err) | ||
| } | ||
| } | ||
|
|
||
| return cfg, nil | ||
| } |
There was a problem hiding this comment.
Less indentation is better
func (s *WebServer) getMarketMakingConfig() ([]*mm.BotConfig, error) {
cfg := []*mm.BotConfig{}
if s.mmCfgPath == "" {
return cfg, nil
}
data, err := os.ReadFile(s.mmCfgPath)
if err == nil {
return cfg, json.Unmarshal(data, &cfg)
}
if os.IsNotExist(err) {
return cfg, nil
}
return nil, fmt.Errorf("error reading file: %w", err)
}
client/mm/mm.go
Outdated
Comment on lines
170
to
171
| noteMtx sync.RWMutex | ||
| noteChans map[uint64]chan core.Notification |
client/webserver/api.go
Outdated
Comment on lines
1739
to
1816
| err = ioutil.WriteFile(s.mmCfgPath, data, 0644) | ||
| if err != nil { | ||
| s.writeAPIError(w, fmt.Errorf("error writing market making config: %v", err)) | ||
| return | ||
| } |
There was a problem hiding this comment.
I think the roles are confused here. Handling config files for the MarketMaker falls squarely under the purview of the MarketMaker. Please consider buck54321/dcrdex@94eec59...buck54321:dcrdex:config-roles
There was a problem hiding this comment.
Looks good. I'm also adding an alternate config file path to MarketMaker.Run so that the RPC server can use whatever file the user specifies.
There was a problem hiding this comment.
I did it this way because when using the RPC server, I think it would be more convenient if you could just specify the path to the config file instead of having to first update the config and then run it. Should Run take an alternate config file path and if it is nil, then the configured one is used.
client/webserver/api.go
Outdated
| return | ||
| } | ||
|
|
||
| err = ioutil.WriteFile(s.mmCfgPath, data, 0644) |
There was a problem hiding this comment.
ioutil is deprecated. Use os.WriteFile.
| tsconfigRootDir: './' | ||
| tsconfigRootDir: __dirname |
There was a problem hiding this comment.
Can you comment on this change?
There was a problem hiding this comment.
This allows eslint to be run from the command line.
martonp
force-pushed
the
mmUI
branch
3 times, most recently
from
3205c06 to
b57a3aa
Compare
buck54321
approved these changes
Sep 17, 2023
client/asset/interface.go
Outdated
Comment on lines
335
to
339
| QuoteAssetOnly bool `json:"quoteAssetOnly"` | ||
| // DependsOn is the key of another config option that if is set to true, | ||
| // this config option will be shown. | ||
| DependsOn string `json:"dependsOn"` | ||
| Range *XYRange `json:"range"` |
There was a problem hiding this comment.
Instead of adding QuoteAssetOnly here, did you consider making WalletDefinition.MultiFundingOpts []*ConfigOption into an []*OrderOption instead? OrderOption already has a Range field, and QuoteAssetOnly is an order-specific configuration option that would fit nicely there.
client/webserver/site/src/js/mm.ts
Outdated
| this.updateProgramDiv(tmpl, report) | ||
| this.programs[report.programID] = Object.assign({ tmpl, div }, report) | ||
| return div | ||
| console.log(JSON.stringify(filteredMkts)) |
|
If I navigate to stack traceIf I navigate to
Maybe I am doing something wrong? I forgot how to navigate to these pages in the UI... |
|
@JoeGruffins I'm unable to reproduce this.. works fine for me with the same build ID. The market making tab is in the hamburger menu, and you get to the |
buck54321
reviewed
Sep 18, 2023
client/webserver/webserver.go
Outdated
Comment on lines
540
to
546
| apiAuth.Post("/startmarketmaking", s.apiStartMarketMaking) | ||
| apiAuth.Post("/stopmarketmaking", s.apiStopMarketMaking) | ||
| apiAuth.Get("/marketmakingconfig", s.apiMarketMakingConfig) | ||
| apiAuth.Post("/updatemarketmakingconfig", s.apiUpdateMarketMakingConfig) | ||
| apiAuth.Post("/removemarketmakingconfig", s.apiRemoveMarketMakingConfig) | ||
| apiAuth.Get("/marketmakingstatus", s.apiMarketMakingStatus) | ||
| apiAuth.Post("/marketreport", s.apiMarketReport) |
There was a problem hiding this comment.
Here's the cause of @JoeGruffins panic. No --experimental flag, no market maker, but he manually navigated to /mm. Not something that a normal user would think to do. be we shouldn't be adding these endpoints if --experimental is not on anyway.
There was a problem hiding this comment.
Oh sorry, that is it. Will build run with the experimental flag.
JoeGruffins
approved these changes
Sep 19, 2023
There was a problem hiding this comment.
Running with the --experimental flag now.
If I navigate to /mmsettings without a token it will still look like the odd page posted above.
Also maybe the cancel on the /mmsettings screen would be better as back? After you change the settings you want to go back and not cancel.
It doesn't seem to save when I unclick "Empty Market Rate"
The delete x for entire bots could use a confirmation dialog imo.
Updates the market making UI to reflect the updated way in which market making works. A JSON configuration file, the same that would be used by the RPC server, is stored on disk, and the UI gives users an interactive way to read/update the config file, and also to start/stop market making. The updated UI includes two screens, `/mm` and `/mmsettings`. `/mm` allows users to see each of the market for which a market maker is configured, allows the user to add/remove market makers, and links to the `/mmsettings` page of each market maker. The `/mmsettings` page allows users to configure each setting of each of the market makers. The markets page is also updated to not allow users to place orders manually while market making is running.
JoeGruffins
reviewed
Sep 21, 2023
There was a problem hiding this comment.
Getting a panic when not running with experimental. This time just navigating to a market, not messing with the new endpoints:
panic
2023/09/21 14:35:24 http: panic serving 127.0.0.1:39028: runtime error: slice bounds out of range [-1:]
goroutine 2788 [running]:
net/http.(*conn).serve.func1()
/usr/lib/go/src/net/http/server.go:1868 +0xb9
panic({0x19e5720?, 0xc0013a1170?})
/usr/lib/go/src/runtime/panic.go:920 +0x270
github.com/go-chi/chi/v5/middleware.prettyStack.decorateFuncCallLine({}, {0xc001ad486c, 0x1f}, 0xd6?, 0x8)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:130 +0x525
github.com/go-chi/chi/v5/middleware.prettyStack.decorateLine({}, {0xc001ad486c?, 0x11e9?}, 0xd0?, 0x1?)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:106 +0x151
github.com/go-chi/chi/v5/middleware.prettyStack.parse({}, {0xc001afc000, 0x11e9, 0xc00094d278?}, {0x1855260, 0x313d860})
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:89 +0x3d0
github.com/go-chi/chi/v5/middleware.PrintPrettyStack({0x1855260, 0x313d860})
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:46 +0x3b
github.com/go-chi/chi/v5/middleware.(*defaultLogEntry).Panic(0x2?, {0x1855260?, 0x313d860?}, {0xc0016b1860?, 0x3?, 0xc0013a1143?})
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/logger.go:165 +0x25
github.com/go-chi/chi/v5/middleware.Recoverer.func1.1()
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:28 +0xc8
panic({0x1855260?, 0x313d860?})
/usr/lib/go/src/runtime/panic.go:914 +0x21f
decred.org/dcrdex/client/mm.(*MarketMaker).Running(...)
/home/joe/git/dcrdex/client/mm/mm.go:213
decred.org/dcrdex/client/webserver.(*WebServer).apiMarketMakingStatus(0xc0008ca0c0, {0x7fd158505078, 0xc0017d3480}, 0x1000000000001?)
/home/joe/git/dcrdex/client/webserver/api.go:1854 +0x25
net/http.HandlerFunc.ServeHTTP(0xc0008ca0c0?, {0x7fd158505078?, 0xc0017d3480?}, 0x838457?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
decred.org/dcrdex/client/webserver.(*WebServer).rejectUnauthed-fm.(*WebServer).rejectUnauthed.func1({0x7fd158505078, 0xc0017d3480}, 0x1?)
/home/joe/git/dcrdex/client/webserver/middleware.go:133 +0x68
net/http.HandlerFunc.ServeHTTP(0xc0007a7d60?, {0x7fd158505078?, 0xc0017d3480?}, 0xc001a95370?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5.(*ChainHandler).ServeHTTP(0x1844760?, {0x7fd158505078?, 0xc0017d3480?}, 0xc0013a1140?)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/chain.go:31 +0x26
github.com/go-chi/chi/v5.(*Mux).routeHTTP(0xc0005d8f00, {0x7fd158505078, 0xc0017d3480}, 0xc001a9d900)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/mux.go:437 +0x1f4
net/http.HandlerFunc.ServeHTTP(0x0?, {0x7fd158505078?, 0xc0017d3480?}, 0x45979c?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5/middleware.AllowContentType.func1.1({0x7fd158505078?, 0xc0017d3480?}, 0xc0013a1141?)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/content_type.go:31 +0x110
net/http.HandlerFunc.ServeHTTP(0xc00094d7d0?, {0x7fd158505078?, 0xc0017d3480?}, 0xc00094d7b0?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5.(*Mux).ServeHTTP(0xc0005d8f00, {0x7fd158505078, 0xc0017d3480}, 0xc001a9d900)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/mux.go:71 +0x356
github.com/go-chi/chi/v5.(*Mux).Mount.func1({0x7fd158505078, 0xc0017d3480}, 0xc001a9d900)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/mux.go:312 +0x1bb
net/http.HandlerFunc.ServeHTTP(0x1844760?, {0x7fd158505078?, 0xc0017d3480?}, 0xc0013ad5f4?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5.(*Mux).routeHTTP(0xc0008a0360, {0x7fd158505078, 0xc0017d3480}, 0xc001a9d900)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/mux.go:437 +0x1f4
net/http.HandlerFunc.ServeHTTP(0xc0015a4928?, {0x7fd158505078?, 0xc0017d3480?}, 0x17bc1a0?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5/middleware.Recoverer.func1({0x7fd158505078?, 0xc0017d3480?}, 0xc001aad0e0?)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/recoverer.go:37 +0x78
net/http.HandlerFunc.ServeHTTP(0xf8?, {0x7fd158505078?, 0xc0017d3480?}, 0x1b8ecd1?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
decred.org/dcrdex/client/webserver.(*WebServer).securityMiddleware-fm.(*WebServer).securityMiddleware.func1({0x7fd158505078, 0xc0017d3480}, 0xc001aad100?)
/home/joe/git/dcrdex/client/webserver/middleware.go:34 +0x169
net/http.HandlerFunc.ServeHTTP(0xc001a9d800?, {0x7fd158505078?, 0xc0017d3480?}, 0x30?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
decred.org/dcrdex/client/webserver.New.RequestLogger.func3.1({0x25875a8, 0xc0013bf500}, 0xc001a9d800)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/middleware/logger.go:57 +0x16d
net/http.HandlerFunc.ServeHTTP(0x258a840?, {0x25875a8?, 0xc0013bf500?}, 0x3140330?)
/usr/lib/go/src/net/http/server.go:2136 +0x29
github.com/go-chi/chi/v5.(*Mux).ServeHTTP(0xc0008a0360, {0x25875a8, 0xc0013bf500}, 0xc001a9d700)
/home/joe/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.1/mux.go:88 +0x315
net/http.serverHandler.ServeHTTP({0xc001aad050?}, {0x25875a8?, 0xc0013bf500?}, 0x6?)
/usr/lib/go/src/net/http/server.go:2938 +0x8e
net/http.(*conn).serve(0xc001aa2d80, {0x258a808, 0xc0008760c0})
/usr/lib/go/src/net/http/server.go:2009 +0x5f4
created by net/http.(*Server).Serve in goroutine 80
/usr/lib/go/src/net/http/server.go:3086 +0x5cb
|
@JoeGruffins Sorry I forgot to comment, the configuration of the config file changed since the Arbitrage strategy was added. Try deleting the config file.
I fixed the other issues, but not this one yet. I definitely agree with it though, and I'll come back to it once @buck54321 gives his more detailed UI review. |
buck54321
approved these changes
Sep 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the market making UI to reflect the updated way in which market making works. A JSON configuration file, the same that would be used by the RPC server, is stored on disk, and the UI gives users an interactive way to read/update the config file, and also to start/stop market making.
The updated UI includes two screens,
/mmand/mmsettings./mmallows users to see each of the market for which a market maker is configured, allows the user to add/remove market makers, and links to the/mmsettingspage of each market maker. The/mmsettingspage allows users to configure each setting of each of the market makers.The markets page is also updated to not allow users to place orders manually while market making is running.