exchanges: Add dynamic function checking to determine exchange functionality #1619

shazbert · 2024-08-18T04:37:58Z

PR Description

Adds AutomaticPreFlightCheck function to determine functionality support between protocols and asset types.
This is a POC for eligibility testing. This is backwards compatible and constricted to gateio for initial documentation output.
This attempts to standardise what is available across exchange implementations between functionality for a quicker decision on what endpoints to use and dynamically scale up and down depending on actual support. Specifically started with trading endpoints to keep things simple.

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and
also consider improving test coverage whilst working on a certain feature or package.

go test ./... -race
golangci-lint run
Test X

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally and on Github Actions with my changes
Any dependent changes have been merged and published in downstream modules

codecov · 2024-08-18T04:43:12Z

Codecov Report

Attention: Patch coverage is 46.61017% with 63 lines in your changes missing coverage. Please review.

Project coverage is 36.39%. Comparing base (225429b) to head (40a5397).
Report is 106 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/documentation/documentation.go	0.00%	42 Missing ⚠️
exchanges/preflight.go	83.87%	3 Missing and 7 partials ⚠️
exchanges/protocol/features.go	0.00%	8 Missing ⚠️
engine/engine.go	50.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1619      +/-   ##
==========================================
+ Coverage   36.36%   36.39%   +0.03%     
==========================================
  Files         422      424       +2     
  Lines      183105   183221     +116     
==========================================
+ Hits        66588    66692     +104     
- Misses     108475   108481       +6     
- Partials     8042     8048       +6

Files with missing lines	Coverage Δ
engine/engine.go	`39.71% <50.00%> (+<0.01%)`	⬆️
exchanges/protocol/features.go	`0.00% <0.00%> (ø)`
exchanges/preflight.go	`83.87% <83.87%> (ø)`
cmd/documentation/documentation.go	`0.00% <0.00%> (ø)`

... and 16 files with indirect coverage changes

github-actions · 2025-01-13T00:01:51Z

This PR is stale because it has been open 30 days with no activity. Please provide an update on the progress of this PR.

github-actions · 2025-02-13T00:01:36Z

This PR is stale because it has been open 30 days with no activity. Please provide an update on the progress of this PR.

gloriousCode

Seems like a worthwhile feature to expand across the exchanges now. I like automation

	var supported bool
	if butts, ok := b.ProtocolCapabilities[protocol.Target{
		Asset:    asset.Spot,
		Protocol: protocol.REST,
	}]; ok {
		supported = butts.AccountBalance
	}

exchanges/protocol/features.go

exchanges/preflight.go

exchanges/gateio/README.md

exchanges/exchange_test.go

…ration

gloriousCode

Nicely done with the revamp. Looks like BTC markets needs a looky and then you're good

exchanges/btcmarkets/README.md

gloriousCode

tHANKS

exchanges/supported_functionality_test.go

engine/engine.go

Co-authored-by: Scott <[email protected]>

gloriousCode

tACK

gbjk

Good work 👍

At first I was skeptical about the complexity of using reflect, and how it shouldn't be so hard for us to just do this.
But then I don't see a better way across all exchanges and assets with what we currently have.

Naming was my biggest initial thing.

I definitely thing we shouldn't pollute runtime Base fields with this when it's only need for docs; Correct me if I'm wrong.

gbjk · 2025-03-20T02:18:52Z

cmd/documentation/documentation.go

+	Features        []ProtocolFunctionality
+}
+
+// ProtocolFunctionality defines a protocol feature set
+type ProtocolFunctionality struct {
+	Protocol string
+	Assets   []AssetFunctionality
+}
+
+func (a ProtocolFunctionality) String() string {
+	return a.Protocol
+}
+
+// AssetFunctionality defines an asset feature set
+type AssetFunctionality struct {
+	Asset   string
+	Methods []MethodNameAndOperational
+}
+
+func (a AssetFunctionality) String() string {
+	return a.Asset
+}
+
+// MethodNameAndOperational defines a method name and if it is operational
+type MethodNameAndOperational struct {
+	MethodName  string
+	Operational bool
+}
+
+// String returns the method name
+func (m MethodNameAndOperational) String() string {
+	return m.MethodName


🐒

I think the naming could use some revision here. MethodNameAndOperational should just be a Method

MethodNameAndOperational.MethodName should just be Method.Name

Operational feels like another term we've already used, like "Supported" ?

Method and Features seems to be overloaded terms, but I think that's okay.

Should these really be slices instead of maps; Feels like we're hiding the unique relationship and forcing a lookup usage later

Suggested change

Features []ProtocolFunctionality

}

// ProtocolFunctionality defines a protocol feature set

type ProtocolFunctionality struct {

Protocol string

Assets []AssetFunctionality

}

func (a ProtocolFunctionality) String() string {

return a.Protocol

}

// AssetFunctionality defines an asset feature set

type AssetFunctionality struct {

Asset string

Methods []MethodNameAndOperational

}

func (a AssetFunctionality) String() string {

return a.Asset

}

// MethodNameAndOperational defines a method name and if it is operational

type MethodNameAndOperational struct {

MethodName string

Operational bool

}

// String returns the method name

func (m MethodNameAndOperational) String() string {

return m.MethodName

AvailableMethods ProtocolMethods

}

type ProtocolMethods map[Protocol]AssetMethods

type AssetMethods map[Asset]MethodSupported

type Asset string // Or asset.Item?

type MethodSupported map[Method]bool

type Method string

gbjk · 2025-03-20T02:20:55Z

cmd/documentation/documentation.go

@@ -485,13 +523,15 @@ func GetContributorList(repo string, verbose bool) ([]Contributor, error) {

 // GetDocumentationAttributes returns specific attributes for a file template
 func GetDocumentationAttributes(packageName string, contributors []Contributor) Attributes {
+	name := GetPackageName(packageName, false)


Feels like we have n+ things using public methods.
Could we make a type Package and then have methods on it for Functionaliy, GoDocURL, etc ?

gbjk · 2025-03-20T02:25:41Z

cmd/documentation/documentation.go

@@ -509,6 +549,48 @@ func GetPackageName(name string, capital bool) string {
 	return newStrings[i]
 }

+// GetFunctionality returns a dynamic list of supported functionality for a specific asset type.


Suggested change

// GetFunctionality returns a dynamic list of supported functionality for a specific asset type.

// GetFunctionality returns a list of supported functionality grouped by protocol and asset type

gbjk · 2025-03-20T02:28:19Z

exchanges/supported_functionality.go

+// GenerateSupportedFunctionality analyzes the exchange's supported asset types and protocols, returning a set of dynamically
+// discovered features (ProtocolCapabilities) that the exchange supports. This process is based on the exchange's
+// specific implementation of wrapper functions.
+func GenerateSupportedFunctionality(exch IBotExchange) protocol.FunctionalitySet {


Suggested change

func GenerateSupportedFunctionality(exch IBotExchange) protocol.FunctionalitySet {

func GetSupportedFunctionality(exch IBotExchange) protocol.FunctionalitySet {

gbjk · 2025-03-20T10:16:36Z

cmd/documentation/documentation.go

@@ -509,6 +549,48 @@ func GetPackageName(name string, capital bool) string {
 	return newStrings[i]
 }

+// GetFunctionality returns a dynamic list of supported functionality for a specific asset type.
+func GetFunctionality(name string) []ProtocolFunctionality {


Suggested change

func GetFunctionality(name string) []ProtocolFunctionality {

func GetFeatures(name string) FeatureMap {

gbjk · 2025-03-20T10:30:14Z

cmd/documentation/documentation.go

@@ -437,7 +471,11 @@ func GetProjectDirectoryTree(c *Config) ([]string, error) {
 // GetTemplateFiles parses and returns all template files in the documentation
 // tree
 func GetTemplateFiles() (*template.Template, error) {
-	tmpl := template.New("")
+	tmpl := template.New("").Funcs(template.FuncMap{
+		"contains":   contains,


Considered using sprig.has ?

gbjk · 2025-03-20T10:30:45Z

cmd/documentation/documentation.go

-	tmpl := template.New("")
+	tmpl := template.New("").Funcs(template.FuncMap{
+		"contains":   contains,
+		"append":     _append,


Considered using sprig.append ?

gbjk · 2025-03-20T10:36:45Z

cmd/documentation/sub_templates/features.tmpl

+{{- /* Collect all unique asset types */ -}}
+{{- range $asset := $feature.Assets }}
+    {{- $assetTypes = append $assetTypes $asset.Asset }}
+    {{- range $method := $asset.Methods }}
+        {{- if not (contains $methodNames $method.MethodName) }}
+            {{- $methodNames = append $methodNames $method.MethodName }}
+        {{- end }}
+    {{- end }}
+{{- end }}
+
+| Method Name |{{range $asset := $assetTypes}} {{$asset}} |{{end}}
+|------------|{{range $asset := $assetTypes}}---|{{end}}
+
+{{- /* Iterate over all unique method names */ -}}
+{{- range $methodName := $methodNames }}
+| {{$methodName}} |
+  {{- range $asset := $assetTypes }}
+    {{- $found := false }}
+    {{- range $lookup := $feature.Assets }}
+      {{- if eq $lookup.Asset $asset }}
+        {{- range $m := $lookup.Methods }}
+          {{- if eq $m.MethodName $methodName }}
+            {{- if $m.Operational }} 🟢 {{else}} 🚫 {{end}}
+            {{- $found = true }}
+          {{- end }}
+        {{- end }}
+      {{- end }}
+    {{- end }}
+    {{- if not $found }} 🚫 {{end}} |
+  {{- end }}
+{{- end }}


This feels like a lot of logic in the template, because of the state vars you've had to create, etc.
I feel like you should be able to get the asset types, the methods and their status inside a func and have a much simpler text template here.

gbjk · 2025-03-20T10:38:54Z

engine/engine.go

+		return err
+	}
+
+	// Update protocol capabilities with dynamic supported functionality check


Don't think this comment is adding anything :)

Suggested change

// Update protocol capabilities with dynamic supported functionality check

gbjk · 2025-03-20T10:40:43Z

exchanges/exchange_types.go

+	Features       Features
+	// ProtocolCapabilities is a map of supported functionality for each asset type and protocol type. This is mapped in
+	// the supported features check to determine if the method is functional and ready for that asset type.
+	ProtocolCapabilities          protocol.FunctionalitySet


Took me a while to work out why this bothered me:

Capabilities are Features

And ProtocolFeatures is just Features re-envisioned, right?

So should we not just change Features itself?
I know it's a lot more work, but if it's the right thing to do...

Alternative thought is: Do we even need to store the FeatureMap? We're not using it often enough to make it a field on Base, IMO. Maybe we derive it when needed

github-actions · 2025-05-02T00:01:38Z

This PR is stale because it has been open 30 days with no activity. Please provide an update on the progress of this PR.

Add initial POC

f23b57b

shazbert added help wanted low priority This enhancement or update will be implemented at a later date. labels Aug 18, 2024

shazbert requested a review from a team August 18, 2024 04:37

shazbert self-assigned this Aug 18, 2024

linter: fix

40a5397

shazbert mentioned this pull request Dec 12, 2024

HitBTC: Add Subscription Configuration #1639

Merged

1 task

github-actions bot added the stale label Jan 13, 2025

shazbert added 2 commits January 14, 2025 08:32

Merge branch 'master' into pre_flight

2805480

rm fmt print call

5e23cc1

shazbert added review me This pull request is ready for review and removed stale labels Jan 13, 2025

github-actions bot added the stale label Feb 13, 2025

gloriousCode requested changes Feb 13, 2025

View reviewed changes

exchanges/protocol/features.go Outdated Show resolved Hide resolved

exchanges/preflight.go Outdated Show resolved Hide resolved

exchanges/gateio/README.md Outdated Show resolved Hide resolved

exchanges/exchange_test.go Outdated Show resolved Hide resolved

shazbert removed help wanted stale labels Feb 13, 2025

Ryan O'Hara-Reid added 3 commits February 13, 2025 16:09

Merge branch 'master' into pre_flight

ff885b8

glorious: nits

db3a07d

rm unused code

9c66b76

shazbert requested a review from gloriousCode February 14, 2025 02:06

Ryan O'Hara-Reid added 4 commits February 14, 2025 13:14

rm more code

2be9569

Merge branch 'master' into pre_flight

f9d7b25

Don't fetch tradable pairs just set defaults on current supported ope…

67e7be2

…ration

linter: fix

3c0a00f

gloriousCode requested changes Mar 4, 2025

View reviewed changes

exchanges/btcmarkets/README.md Show resolved Hide resolved

Ryan O'Hara-Reid added 2 commits March 5, 2025 12:54

Merge branch 'master' into pre_flight

bc02360

glorious: nits

8342c02

gloriousCode requested changes Mar 5, 2025

View reviewed changes

exchanges/supported_functionality_test.go Outdated Show resolved Hide resolved

engine/engine.go Outdated Show resolved Hide resolved

shazbert and others added 2 commits March 6, 2025 10:52

Update exchanges/supported_functionality_test.go

3b7e781

Co-authored-by: Scott <[email protected]>

Update engine/engine.go

7b2c21d

Co-authored-by: Scott <[email protected]>

gloriousCode approved these changes Mar 6, 2025

View reviewed changes

gloriousCode added the gcrc GloriousCode Review Complete label Mar 6, 2025

gbjk requested changes Mar 20, 2025

View reviewed changes

shazbert removed the review me This pull request is ready for review label Apr 1, 2025

github-actions bot added the stale label May 2, 2025

	// GetFunctionality returns a dynamic list of supported functionality for a specific asset type.
	// GetFunctionality returns a list of supported functionality grouped by protocol and asset type

	func GenerateSupportedFunctionality(exch IBotExchange) protocol.FunctionalitySet {
	func GetSupportedFunctionality(exch IBotExchange) protocol.FunctionalitySet {

	func GetFunctionality(name string) []ProtocolFunctionality {
	func GetFeatures(name string) FeatureMap {

exchanges: Add dynamic function checking to determine exchange functionality #1619

Are you sure you want to change the base?

exchanges: Add dynamic function checking to determine exchange functionality #1619

Uh oh!

Conversation

shazbert commented Aug 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Type of change

How has this been tested

Checklist

Uh oh!

codecov bot commented Aug 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jan 13, 2025

Uh oh!

github-actions bot commented Feb 13, 2025

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gloriousCode left a comment

Choose a reason for hiding this comment

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 2, 2025

Uh oh!

Uh oh!

shazbert commented Aug 18, 2024 •

edited

Loading

codecov bot commented Aug 18, 2024 •

edited

Loading