api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request #2175

mortelli · 2020-05-06T17:53:30Z

This PR is a WIP for the following task:

implement inheriting publisher info via the context.WithValue, construct and lookup the latest update of the pinners feed.

from issue #2161.

Implementation overview

NewRecoveryHook constructor func now also receives a feeds handler as a param, so the actual recovery hook can use it for feeds.
- the handler passed to this constructor is the one created in the NewSwarm function.
prod.getPinners func now extracts a publisher string from the ctx param, gets the content from a feed using the address derived from the publisher, and unmarshals the content feed as targets to construct the trojan chunk.
adds a publisher flag for swarm up to register the publisher in the file manifest
adds a request header for upload with the publisher of the content

Tasks

add tests if possible
resolve TODOs or create new issues for them

…ing-feeds

…ookup and DummyHandler.GetContent funcs prod: change NewRecoveryHook and getPinners funcs handler params to FeedsHandler type

… comment feed package funcs

mortelli · 2020-05-06T22:25:51Z

storage/netstore.go

@@ -99,7 +99,7 @@ type NetStore struct {
 	requestGroup     singleflight.Group
 	RemoteGet        RemoteGetFunc
 	logger           log.Logger
-	recoveryCallback func(ctx context.Context, chunkAddress chunk.Address) error // this is the function callback when a chunk failed to be retrieved
+	recoveryCallback func(ctx context.Context, chunkAddress chunk.Address, publisher string) error // this is the callback to be executed when a chunk fails to be retrieved


publisher may not be a param here, but rather extracted from ctx

it makes sense to extract from context within the recovery function

removed this param, expecting code to be pushed to extract this data from ctx

mortelli · 2020-05-06T22:26:26Z

storage/netstore.go

@@ -167,7 +167,7 @@ func (n *NetStore) Close() error {
 }

 // WithRecoveryCallback allows injecting a callback func on the NetStore struct
-func (n *NetStore) WithRecoveryCallback(f func(ctx context.Context, chunkAddress chunk.Address) error) *NetStore {
+func (n *NetStore) WithRecoveryCallback(f func(ctx context.Context, chunkAddress chunk.Address, publisher string) error) *NetStore {


see previous comment

resolution here

mortelli · 2020-05-06T22:27:35Z

pss/trojan/message.go

@@ -45,6 +45,12 @@ const MaxPayloadSize = 4030

 var hashFunc = storage.MakeHashFunc(storage.BMTHash)

+// RecoveryTopicText is the string used to construct the RecoveryTopic var
+const RecoveryTopicText = "RECOVERY"


we need a separate string here as a const in order to build a feed.Topic var in the getPinners function (which is not of the trojan.Topic type)

this should not be here. trojan pkg not supposed to know about recovery

thanks, moved this to prod

prod/prod.go

mortelli · 2020-05-06T22:34:49Z

storage/feed/handler.go

@@ -31,6 +31,13 @@ import (
 	"github.com/ethersphere/swarm/storage/feed/lookup"
 )

+// GenericHandler is an interface which specifies funcs any feeds handler should use
+type GenericHandler interface {
+	Lookup(ctx context.Context, query *Query) (*cacheEntry, error)


this is the signature as originally defined in the feeds package.

it seems strange to me that the cacheEntry type is not exported, since it is returned in the Lookup function, which is.

mortelli · 2020-05-06T22:47:25Z

prod/prod.go

+	// get feed user from publisher
+	publisherBytes, err := hex.DecodeString(publisher)
+	if err != nil {
+		return nil, ErrPublisher
+	}
+	pubKey, err := crypto.DecompressPubkey(publisherBytes)
+	if err != nil {
+		return nil, ErrPubKey
+	}
+	addr := crypto.PubkeyToAddress(*pubKey)


not entirely sure these are the steps to go from an (assumed) byte string as the publisher, to the actual User address for the feed.

as clarification: this code was adapted from api/act.go:245

looks good but this should defo be done only once instead of publisher, the derived ether address should be put in the context value

agreed, registered in #2181

zelig · 2020-05-07T23:20:07Z

prod/prod.go

-		// TODO: monitor return should
-		if _, err := send(ctx, targets, topic, payload); err != nil {
+		// TODO: returned monitor should be made use of
+		if _, err := send(ctx, targets, trojan.RecoveryTopic, payload); err != nil {


yes, we need the monitor here, I would block this until either parent context is done or monitor shows synced request

this should be done in #2171

mortelli · 2020-05-08T13:50:39Z

how about extracting the non-controversial parts in one or two PRs which we just merge?
* Targets  type in trojan package

* feed testutils and handler interface
then rebase this PR and we figure out contexts and how publisher gets in there etc

added PRs #2178 and #2179 for this.

thank you for the very appropriate suggestion

…d for testing

… publish flag for swarm up command, added header publisher for uploadTar, replace prod with context publisher, updated tests to reflect these changes

prod/prod.go

…ing-feeds # Conflicts: # prod/prod.go # pss/pss_test.go

mortelli · 2020-05-12T12:15:45Z

new related issue: #2180

mortelli

quick overview of the stuff @zelig has not commented on

api/api.go

cmd/swarm/flags.go

api/http/server.go

prod/prod_test.go

mortelli · 2020-05-12T18:35:24Z

new related issue: #2181

mortelli

linter passes, so do tests, and all outstanding issues are documented.

GEFM, but i can't actually approve because i am the author of the PR 😂

prod/prod.go

santicomp2014

LGTM
Addressed all points, this PR has generated some secondary issues:
Some are improvements and will be dealt with to improve the overall design

zelig

looks perfect

zelig · 2020-05-12T20:12:16Z

prod/prod.go

+const RecoveryTopicText = "RECOVERY"
+
+// RecoveryTopic is the topic used for repairing globally pinned chunks
+var RecoveryTopic = trojan.NewTopic("RECOVERY")


here use RecoveryTopicText

whoops, this was left behind when i split the code into 2 more PRs.

fixed in the main branch, thanks

zelig · 2020-05-12T20:15:51Z

prod/prod.go

+		Topic: topic,
+		User:  addr,
+	}
+	query := feed.NewQueryLatest(&fd, lookup.NoClue)


is this always lookup.NoClue? does it not store latest. state to be. used as hint?

as of now, we do not have any idea when the last update to the feed happened, so this is why NoClue is used.

but i have added a note to #2181, we can further optimize it this way i think.

thanks

…tation from PR #2175

mortelli added 7 commits May 5, 2020 14:49

Merge remote-tracking branch 'origin/master' into global-pinning-feeds

8c4e6a7

Merge remote-tracking branch 'origin/global-pinning' into global-pinn…

7a213dc

…ing-feeds

prod: iterate getPinners func

7132166

storage, prod: add publisher param to recovery hook

f25b463

prod: update TestRecoveryHook for new hook

64741c4

prod: iterate handler use in recovery hook construction and invocation

7b6ef91

prod: iterate handler use in recovery hook construction and invocation

ce6e347

mortelli added in progress global pinning experimental implementation of global pinning labels May 6, 2020

mortelli assigned santicomp2014 and mortelli May 6, 2020

mortelli mentioned this pull request May 6, 2020

global pinning: phase 2 #2161

Closed

6 tasks

mortelli added 10 commits May 6, 2020 16:12

feed: add FeedsHandler interface, DummyHandler struct, DummyHandler.L…

ed7e691

…ookup and DummyHandler.GetContent funcs prod: change NewRecoveryHook and getPinners funcs handler params to FeedsHandler type

feed, prod: rename FeedsHandler interface to GenericHandler, move and…

934ac93

… comment feed package funcs

chunk, storage, prod: refine comments

81954b3

prod, pss, trojan: extract RecoveryTopic var

0842aff

swarm: use RecoveryTopic var

2c55d03

prod: add feed error vars

78c188b

prod: fix topic for feed in getPinners func

467473c

prod: iterate tests

3608343

prod: refine error handling in getPinners

9747df5

prod: update error var comments

7e1b979

mortelli commented May 6, 2020

View reviewed changes

prod/prod.go Show resolved Hide resolved

mortelli added 2 commits May 6, 2020 19:31

prod: iterate comments in getPinners func

52dac5b

feed: small refactor to newDummyCacheEntry func

64de461

mortelli commented May 6, 2020

View reviewed changes

zelig reviewed May 7, 2020

View reviewed changes

mortelli added 2 commits May 8, 2020 09:22

storage, prod: remove publisher param from store hook

97dc5cd

swarm, prod, trojan: move recovery topic vars to prod package

0250a5f

santicomp2014 and others added 5 commits May 8, 2020 22:46

api, storage: Publisher added to manifest, remove hardcoded value use…

6195cf7

…d for testing

pss: remove unnecessary dependency to prod package on pss test

931b76e

api,api/client,api/http,cmd/swarm,cmd/swarm-smoke,prod,storage: added…

272df48

… publish flag for swarm up command, added header publisher for uploadTar, replace prod with context publisher, updated tests to reflect these changes

api: remove test debug logs

7fc3df5

api,prod: removed unnecessary publisher default value

a28f1ff

santicomp2014 marked this pull request as ready for review May 11, 2020 21:45

zelig reviewed May 12, 2020

View reviewed changes

prod/prod.go Show resolved Hide resolved

zelig reviewed May 12, 2020

View reviewed changes

prod/prod.go Show resolved Hide resolved

Merge remote-tracking branch 'origin/global-pinning' into global-pinn…

e6f0099

…ing-feeds # Conflicts: # prod/prod.go # pss/pss_test.go

mortelli commented May 12, 2020

View reviewed changes

santicomp2014 added 3 commits May 12, 2020 13:22

api, api/http: fixed len check for publisher and also init value

470ee96

api,prod: removed ctx and clear, moved todo for fallback flag to prod

5c2f9f0

prod: added timeout to ctx in getPinners

61c33d9

mortelli added ready for review and removed in progress labels May 12, 2020

mortelli commented May 12, 2020

View reviewed changes

prod/prod.go Show resolved Hide resolved

mortelli commented May 12, 2020

View reviewed changes

prod/prod.go Show resolved Hide resolved

santicomp2014 approved these changes May 12, 2020

View reviewed changes

zelig changed the title ~~prod, chunk, pss, trojan, feed, storage, swarm: get publisher and query feed for chunk repair request~~ api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request May 12, 2020

zelig approved these changes May 12, 2020

View reviewed changes

zelig merged commit d91b839 into global-pinning May 12, 2020

mortelli deleted the global-pinning-feeds branch May 13, 2020 12:38

mortelli added a commit that referenced this pull request May 26, 2020

main, swarm, http, prod, api, client: undo publisher context implemen…

fd035f5

…tation from PR #2175

mortelli mentioned this pull request May 26, 2020

main, swarm, http, prod, api, client: repair globally pinned content through CLI publisher #2194

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request #2175

api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request #2175

mortelli commented May 6, 2020 •

edited

Loading

mortelli May 6, 2020 •

edited

Loading

zelig May 7, 2020

mortelli May 8, 2020

mortelli May 6, 2020 •

edited

Loading

mortelli May 8, 2020

mortelli May 6, 2020

zelig May 7, 2020

mortelli May 8, 2020

mortelli May 6, 2020

zelig May 7, 2020

mortelli May 6, 2020 •

edited

Loading

zelig May 7, 2020

mortelli May 12, 2020

zelig May 7, 2020

mortelli May 8, 2020

mortelli commented May 8, 2020

mortelli commented May 12, 2020 •

edited

Loading

mortelli left a comment

mortelli commented May 12, 2020

mortelli left a comment

santicomp2014 left a comment

zelig left a comment

zelig May 12, 2020

mortelli May 13, 2020

zelig May 12, 2020

mortelli May 13, 2020

api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request #2175

api, cmd, prod, pss, swarm: get publisher and query feed for chunk repair request #2175

Conversation

mortelli commented May 6, 2020 • edited Loading

Implementation overview

Tasks

mortelli May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mortelli May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mortelli May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mortelli commented May 8, 2020

mortelli commented May 12, 2020 • edited Loading

mortelli left a comment

Choose a reason for hiding this comment

mortelli commented May 12, 2020

mortelli left a comment

Choose a reason for hiding this comment

santicomp2014 left a comment

Choose a reason for hiding this comment

zelig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mortelli commented May 6, 2020 •

edited

Loading

mortelli May 6, 2020 •

edited

Loading

mortelli May 6, 2020 •

edited

Loading

mortelli May 6, 2020 •

edited

Loading

mortelli commented May 12, 2020 •

edited

Loading