Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use federation sync config in syncer #564

Merged
merged 21 commits into from
Oct 12, 2023
Merged

Use federation sync config in syncer #564

merged 21 commits into from
Oct 12, 2023

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Oct 11, 2023

In this PR we modify the simple syncer and the federation envoy such that they use the federation sync configuration.


This change is Reviewable

@ffranr
Copy link
Contributor Author

ffranr commented Oct 11, 2023

In the fed sync config we differentiate between import and export syncing. This PR doesn't currently handle that nuance. We sync if both import and export is enabled.

@ffranr ffranr changed the base branch from federation-sync-config-dep to main October 11, 2023 18:26
@ffranr ffranr marked this pull request as ready for review October 11, 2023 18:26
universe/auto_syncer.go Show resolved Hide resolved
universe/auto_syncer.go Show resolved Hide resolved
universe/auto_syncer.go Outdated Show resolved Hide resolved
universe/auto_syncer.go Outdated Show resolved Hide resolved
universe/auto_syncer.go Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Member

Roasbeef commented Oct 11, 2023

  • Examine export/insert in the rpc calls, reject read for export, opposite for import
  • Only examine insert for the syncer
  • Scratchup config, if only for itests -- --universe.public_access, allows insert+export global -- (--universe.public_issuance, --universe.public_proofs)

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: 7 of 9 files reviewed, 6 unresolved discussions (waiting on @ffranr and @guggero)


rpcserver.go line 3272 at r3 (raw file):

	// TODO(roasbeef): query may return multiple proofs, if allow key to
	// not be fully specified
	proof := proofs[0]

if len(proofs) == 0 {
return universe.ErrNoNUniverseProofFound
}]

Code quote:

	// TODO(roasbeef): query may return multiple proofs, if allow key to
	// not be fully specified
	proof := proofs[0]

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r3.
Reviewable status: 8 of 9 files reviewed, 7 unresolved discussions (waiting on @ffranr and @guggero)


universe/auto_syncer.go line 490 at r3 (raw file):

	// Obtain the general and universe specific federation sync configs.
	syncConfigs, err := f.QuerySyncConfigs(ctx)

Missing error check.

@ffranr ffranr force-pushed the use-sync-config branch 3 times, most recently from f1477f8 to 305962c Compare October 11, 2023 23:48
ffranr and others added 10 commits October 11, 2023 20:03
Before this commit, we would insert it all into the same tree. Instead,
we'll now properly insert them into distinct trees based on the proof
type.
We'll use this in an upcoming test to assert the new sum changes we made
earlier in this commit series.

Note that we just query the tree directly, as we currently don't have
the proper pointers to the leaf and root node.
The top level tree in the issuance multi verse root is just an
accumulator value, so we use a value of 1 here.
At times spew can choke on a data structure like our smt tree. Not sure
why this didn't really care before, but does now. Either way, allows a
test to proceed.
@Roasbeef Roasbeef merged commit e614ff9 into main Oct 12, 2023
14 checks passed
@guggero guggero deleted the use-sync-config branch October 12, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants