feat: add EFP (Ethereum Follow Protocol) integration for wallet #7052
Conversation
caveman-eth
changed the title
jrainville
requested review from
Khushboo-dev-cpp,
dlipicar,
igor-sirotin and
jrainville
jrainville
left a comment
jrainville
left a comment
There was a problem hiding this comment.
Awesome work! Looks very good.
You'll have to rebase and fix some lint issues I think
|
ah, also commits need to follow conventional commit format, eg |
|
@caveman-eth I have sent you an Github invite for collaborator. Please accept and rebase to trigger the CI 🙂 |
Jenkins BuildsClick to see older builds (47)
|
caveman-eth
force-pushed
the
develop
branch
2 times, most recently
from
84ee4b3 to
f957022
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7052 +/- ##
===========================================
- Coverage 59.92% 59.79% -0.13%
===========================================
Files 813 815 +2
Lines 113989 114160 +171
===========================================
- Hits 68306 68261 -45
- Misses 38754 38924 +170
- Partials 6929 6975 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Great, looks like everything is passing now
@igor-sirotin I think everything is right now :) |
services/wallet/following/manager.go
Outdated
|
|
||
| // Manager handles following address operations using EFP providers | ||
| type Manager struct { | ||
| providers []efp.FollowingDataProvider |
There was a problem hiding this comment.
we use a slice of providers for cases where multiple providers can be used to fetch the same type of data. I'm not super well versed in EFP, but it doesn't look like that's something we expect to happen, right?
Moreover, we're just using m.providers[0] in all methods, so making this a slice is misleading, better just make it provider efp.FollowingDataProvider
services/wallet/following/manager.go
Outdated
| ) | ||
|
|
||
| if len(m.providers) == 0 { | ||
| return []efp.FollowingAddress{}, nil |
There was a problem hiding this comment.
feels like we should return an error if the provider is not set and we make a request, otherwise we can't distinguish this from a "true" empty FollowingAddresses result.
services/wallet/service.go
Outdated
|
|
||
| // EFP (Ethereum Follow Protocol) providers | ||
| efpClient := efp.NewClient() | ||
| followingProviders := []efp.FollowingDataProvider{ |
There was a problem hiding this comment.
Check how we conditionally populate the other provider lists https://github.com/status-im/status-go/blob/develop/services/wallet/service.go#L119
Please do the same thing here, we don't want accesses to the provider if thirdpartyServicesEnabled is false
| func NewClient() *Client { | ||
| httpClient := thirdparty.NewHTTPClient( | ||
| thirdparty.WithDetailedTimeouts( | ||
| 5*time.Second, // dialTimeout |
There was a problem hiding this comment.
these seem to be opinionated default values without any way of overriding them. Perhaps pass the httpClient as a constructor parameter instead?
| const baseURL = "https://api.ethfollow.xyz/api/v1" | ||
|
|
||
| const ( | ||
| requestDelay = 100 * time.Millisecond |
dlipicar
left a comment
dlipicar
left a comment
There was a problem hiding this comment.
Thanks for your contribution! A couple minor remarks added
igor-sirotin
left a comment
igor-sirotin
left a comment
There was a problem hiding this comment.
I agree with comments from @dlipicar, otherwise looks good! Thanks for your contribution!
I've left some minor suggestions, no blockers from my side.
| func setupTest(t *testing.T, response []byte) (*httptest.Server, func()) { | ||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(200) | ||
| _, err := w.Write(response) | ||
| if err != nil { | ||
| return | ||
| } | ||
| })) | ||
|
|
||
| return srv, func() { | ||
| srv.Close() | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe implement a test suite, not to repeat this call in every test?
type EFPClientTestSuite struct {
suite.Suite
}
func (s *EFPClientTestSuite) SetupTest() {
// setup
}
func (s *EFPClientTestSuite) TeardownTest() {
// cleanup
}
|
@dlipicar and @igor-sirotin thanks for reviewing. |
igor-sirotin
left a comment
igor-sirotin
left a comment
There was a problem hiding this comment.
Thanks for the fixes 👍
|
I have updated the branch with latest upstream commits, and resolved a status-go conflict. |
|
@caveman-eth let's make sure status-im/status-desktop#19195 is green and approved, I'll merge this one then |
Introduces Ethereum Follow Protocol (EFP) integration for fetching following addresses and stats. Adds a following manager, EFP client, and related API endpoints, along with tests for both manager and client functionality.
Changed the go:generate directive to explicitly use 'go tool mockgen' instead of just 'mockgen' for generating mocks. This may improve compatibility or clarity in environments where the tool is not globally installed.
Refactored the following.Manager to accept a single EFP provider and logger instead of a slice of providers. Updated initialization in service.go and all related tests. Adjusted EFP client to require an HTTP client on construction, improving dependency injection and testability.
5 participants
Adds support for fetching and managing Ethereum Follow Protocol (EFP) followings in the wallet service
Relating to: status-im/status-desktop#18686
and status-desktop PR: status-im/status-desktop#19195
Screenshot:
Complete video demo:
https://streamable.com/3y4s1f
Changes:
New EFP Client (
services/wallet/thirdparty/efp/):/searchFollowingendpointFollowing Manager (
services/wallet/following/):RPC API (
services/wallet/api.go):wallet_GetFollowingAddresses(userAddress, search, limit, offset)- fetch following addresses with optional search and paginationwallet_GetFollowingStats(userAddress)- get total following countTests:
client_test.go- 6 unit tests for HTTP client (pagination, search, stats, error handling)manager_test.go- 5 unit tests for manager (success cases, provider errors, edge cases)API Endpoints Used
GET /users/{address}/following?include=ens&limit={limit}&offset={offset}&sort=latestGET /users/{address}/searchFollowing?include=ens&sort=latest&term={search}GET /users/{address}/stats