endpoint.Service: Bind multiple ephemeral identities at the same time #1009 #1033

adecaro · 2025-08-26T06:27:45Z

The goal of this PR is to allow the binding of multiple identities at the same time so to same in the number of DB connections when binding multiple identities. This will be used in the token-sdk.

This PR is linked to FTS's PR hyperledger-labs/fabric-token-sdk#1245

platform/view/services/storage/driver/sql/sqlite/binding.go

platform/view/services/endpoint/service_test.go

mbrandenburger

Thanks again for addressing my comments.

platform/view/services/storage/driver/sql/postgres/binding_test.go

platform/view/services/storage/driver/sql/sqlite/binding_test.go

platform/view/services/storage/driver/sql/sqlite/binding.go

Signed-off-by: [email protected] <[email protected]>

Signed-off-by: Angelo De Caro <[email protected]>

Signed-off-by: [email protected] <[email protected]>

2. fixed postgres test Signed-off-by: [email protected] <[email protected]>

Signed-off-by: [email protected] <[email protected]>

mbrandenburger

Thanks for this PR! It is in a good shape already. See below a few minor comments to address. Thanks again!

platform/common/utils/collections/slices.go

mbrandenburger · 2025-11-05T09:20:18Z

platform/fabric/sdk/dig/sdk_test.go

+	sss := sdk.WithBool("fabric.enabled", true)
+	err := sdk.DryRunWiring(NewFrom, sss)
+	assert.NoError(t, err)


Is this change really needed?

mbrandenburger · 2025-11-05T09:22:39Z

platform/view/services/storage/driver/sql/postgres/binding.go

-		logger.DebugfContext(ctx, "Id [%s] is an unregistered long term ID", longTerm.UniqueID())
+
+func (db *BindingStore) PutBindings(ctx context.Context, longTerm view.Identity, ephemerals ...view.Identity) error {
+	if len(ephemerals) == 0 {


should we also check that longTerm is not nil?

platform/view/services/endpoint/service.go

platform/view/services/storage/driver/sql/postgres/binding_test.go

platform/view/services/storage/driver/sql/sqlite/binding.go

mbrandenburger · 2025-11-05T10:36:06Z

platform/view/services/storage/driver/sql/sqlite/binding_test.go

+	// Input identities
+	longTerm := view.Identity("long")
+	e1 := view.Identity("eph1")
+	e2 := view.Identity("eph2")
+
+	// Check that store does not have bindings for e1 and e2
+	lt, err := db.GetLongTerm(ctx, e1)
+	require.NoError(t, err)
+	require.ElementsMatch(t, len(lt), 0)
+	lt, err = db.GetLongTerm(ctx, e2)
+	require.NoError(t, err)
+	require.ElementsMatch(t, len(lt), 0)
+
+	// Create new bindings
+	err = db.PutBindings(ctx, longTerm, e1, e2)
+	require.NoError(t, err)
+
+	// Check that the bindings where correctly written
+	lt, err = db.GetLongTerm(ctx, e1)
+	require.NoError(t, err)
+	require.ElementsMatch(t, lt, longTerm)
+
+	lt, err = db.GetLongTerm(ctx, e2)
+	require.NoError(t, err)
+	require.ElementsMatch(t, lt, longTerm)


The test looks more or less identical to the postgres/binding_test.go

Let's move the actual test case to common/binding_test.go into a test method e.g.,
RunPutBindingTests, which takes as input a database abstraction.

In this file here we just instantiate the db and call common.RunPutBindingTests(t, db). We do the same for the postgres test.

WDYT?

With the latest changes you moved the actual binding code to common. Please consider doing the same for the actual test cases.

mbrandenburger · 2025-11-05T10:36:31Z

platform/view/services/storage/driver/sql/sqlite/binding.go

+	// Build single INSERT with multiple VALUES
+	query := fmt.Sprintf(`INSERT INTO %s (ephemeral_hash, long_term_id) VALUES `, db.table)
+
+	args := []interface{}{}


Suggested change

args := []interface{}{}

var args := []any

platform/view/services/storage/driver/sql/common/binding.go:116:13: []any (type) is not an expression

sorry - this was a sloppy suggestion. it should be var args []any.

platform/view/services/storage/driver/sql/postgres/binding.go

Signed-off-by: [email protected] <[email protected]>

mbrandenburger · 2025-11-06T15:09:30Z

platform/view/services/storage/driver/sql/common/binding.go

+	if len(ephemerals) > 1000 {
+		return errors.Errorf("Too many ephemerals (%d). Max allowed is 1000", len(ephemerals))
+	}


Please update the go docs for this function to clearly describe that here is a limit.

mbrandenburger · 2025-11-06T15:11:29Z

platform/fabric/sdk/dig/sdk_test.go

-func TestWiring(t *testing.T) {
-	assert.NoError(t, sdk.DryRunWiring(NewFrom, sdk.WithBool("fabric.enabled", true)))
-}


Sorry, I did not mean to remove this test. I meant to just revert the changes to it. Let's keep it. It's a good test.

mbrandenburger · 2025-11-06T15:14:09Z

platform/view/services/storage/driver/sql/sqlite/binding_test.go

+	// Input identities
+	longTerm := view.Identity("long")
+	e1 := view.Identity("eph1")
+	e2 := view.Identity("eph2")
+
+	// Check that store does not have bindings for e1 and e2
+	lt, err := db.GetLongTerm(ctx, e1)
+	require.NoError(t, err)
+	require.ElementsMatch(t, len(lt), 0)
+	lt, err = db.GetLongTerm(ctx, e2)
+	require.NoError(t, err)
+	require.ElementsMatch(t, len(lt), 0)
+
+	// Create new bindings
+	err = db.PutBindings(ctx, longTerm, e1, e2)
+	require.NoError(t, err)
+
+	// Check that the bindings where correctly written
+	lt, err = db.GetLongTerm(ctx, e1)
+	require.NoError(t, err)
+	require.ElementsMatch(t, lt, longTerm)
+
+	lt, err = db.GetLongTerm(ctx, e2)
+	require.NoError(t, err)
+	require.ElementsMatch(t, lt, longTerm)


With the latest changes you moved the actual binding code to common. Please consider doing the same for the actual test cases.

mbrandenburger · 2025-11-06T15:18:28Z

platform/view/services/storage/driver/sql/postgres/binding_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestPutBindingsMultipleEphemeralsPostgres(t *testing.T) {


What about adding one case to test our limit :)

adecaro self-assigned this Aug 26, 2025

adecaro added improvement perf labels Aug 26, 2025

adecaro linked an issue Aug 26, 2025 that may be closed by this pull request

endpoint.Service: Bind multiple ephemeral identities at the same time #1009

Open

adecaro added this to the Q3 milestone Aug 26, 2025

adecaro mentioned this pull request Aug 27, 2025

unite multiple bind between ephemaral IDs and long-term IDs #1020

Closed

adecaro requested a review from mbrandenburger August 27, 2025 11:58

mbrandenburger mentioned this pull request Sep 2, 2025

FSC Update: endpoint.Service: Bind multiple ephemeral identities at the same time #1009 hyperledger-labs/fabric-token-sdk#1214

Closed

AkramBitar assigned HayimShaul Sep 3, 2025

mbrandenburger reviewed Sep 3, 2025

View reviewed changes

platform/view/services/storage/driver/sql/sqlite/binding.go Outdated Show resolved Hide resolved

platform/view/services/endpoint/service_test.go Outdated Show resolved Hide resolved

HayimShaul force-pushed the 1009-unite-multiple-binds-2 branch from 312e46b to 5625f77 Compare September 3, 2025 14:04

HayimShaul requested a review from mbrandenburger September 3, 2025 14:06

mbrandenburger reviewed Sep 4, 2025

View reviewed changes

HayimShaul force-pushed the 1009-unite-multiple-binds-2 branch from 674d885 to 2cd3a22 Compare September 8, 2025 06:57

HayimShaul unassigned adecaro Sep 8, 2025

AkramBitar self-requested a review September 8, 2025 11:49

HayimShaul force-pushed the 1009-unite-multiple-binds-2 branch from 585de5d to 406634f Compare October 29, 2025 16:30

[email protected] and others added 13 commits November 3, 2025 04:25

unite multiple bind between ephemaral IDs and long-term IDs

0f51e12

Signed-off-by: [email protected] <[email protected]>

auto generated

3050658

Signed-off-by: [email protected] <[email protected]>

fixed test

ee3127d

Signed-off-by: [email protected] <[email protected]>

Fixing Backend interface

ef21e8b

Signed-off-by: [email protected] <[email protected]>

function names

7b53431

Signed-off-by: [email protected] <[email protected]>

auto-gen code

65bf5e7

Signed-off-by: [email protected] <[email protected]>

breaking down the code

48affab

Signed-off-by: [email protected] <[email protected]>

auto-gen files

3f271a9

Signed-off-by: [email protected] <[email protected]>

change Bind in interface

de74ebb

Signed-off-by: [email protected] <[email protected]>

fix interface

442f601

Signed-off-by: [email protected] <[email protected]>

fix lint errors

87a5194

Signed-off-by: [email protected] <[email protected]>

cleanup

41f9983

Signed-off-by: Angelo De Caro <[email protected]>

don't panic if binding and no ephemeral ids were given

e867d19

Signed-off-by: [email protected] <[email protected]>

[email protected] and others added 5 commits November 3, 2025 04:25

1. moved common test code to common

f397f01

2. fixed postgres test Signed-off-by: [email protected] <[email protected]>

remove obsolete code

89f11b4

Signed-off-by: [email protected] <[email protected]>

name fix

e055350

Signed-off-by: [email protected] <[email protected]>

add integration test for PutBindings

fc8eb06

Signed-off-by: [email protected] <[email protected]>

removed redundent tests

efa9438

Signed-off-by: [email protected] <[email protected]>

HayimShaul force-pushed the 1009-unite-multiple-binds-2 branch from 406634f to efa9438 Compare November 3, 2025 09:26

mbrandenburger requested review from HayimShaul and mbrandenburger November 4, 2025 14:50

adecaro mentioned this pull request Nov 5, 2025

use new FSC hyperledger-labs/fabric-token-sdk#1245

Open

mbrandenburger requested changes Nov 5, 2025

View reviewed changes

[email protected] added 10 commits November 5, 2025 08:52

remove slices.FilterSlice

9f9b82c

Signed-off-by: [email protected] <[email protected]>

remove redundant test

b5b79cd

Signed-off-by: [email protected] <[email protected]>

check for nil longTerm id

f116416

Signed-off-by: [email protected] <[email protected]>

return early if empty list to bind

bf2d3ae

Signed-off-by: [email protected] <[email protected]>

function names to indicate database in test

ed8eca9

Signed-off-by: [email protected] <[email protected]>

move common function to common file

ad5b2e6

Signed-off-by: [email protected] <[email protected]>

style

7959996

Signed-off-by: [email protected] <[email protected]>

style

005f781

Signed-off-by: [email protected] <[email protected]>

simplify query build

b9790bb

Signed-off-by: [email protected] <[email protected]>

PutBindings: check for too many ephemeral ids

33e95eb

Signed-off-by: [email protected] <[email protected]>

HayimShaul requested a review from mbrandenburger November 6, 2025 09:27

mbrandenburger requested changes Nov 6, 2025

View reviewed changes

endpoint.Service: Bind multiple ephemeral identities at the same time #1009 #1033

Are you sure you want to change the base?

endpoint.Service: Bind multiple ephemeral identities at the same time #1009 #1033

Conversation

adecaro commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbrandenburger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mbrandenburger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adecaro commented Aug 26, 2025 •

edited

Loading