dlog v1 finalization: enforce at least one issuer #1010

adecaro · 2025-03-23T07:53:20Z

We start by adding the check that at least one issuer identity is specified in the Validate function of the public parameters. From there, we make sure the rest still works.
Many of the integration tests were assuming that anyone could issue, therefore we had to change the test to update the public parameters with the identities of who needed to issue.

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

update public params during test when needed LocalMembership should not discard identities not in the target list, just assign a low priority Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

support for fabtoken pp Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

alexandrosfilios · 2025-03-26T11:52:45Z

integration/token/fungible/support.go

+func PrepareUpdatedPublicParams(network *integration.Infrastructure, auditor string, networkName string) []byte {
+	tms := GetTMSByNetworkName(network, networkName)
+	auditorId := GetAuditorIdentity(tms, auditor)
+	issuerId := GetIssuerIdentity(tms, "newIssuer")


Gives a bit more clarity if we pass the issuer id as a parameter too.

alexandrosfilios · 2025-03-26T11:57:38Z

token/core/fabtoken/v1/driver/driver.go

+	// log information about the public params
+	pp := publicParamsManager.PublicParams()
+	logger.Infof("new token driver for tms id [%s] with label and version [%s:%s]", tmsID, pp.Identifier(), pp.Version())
+	for _, issuer := range pp.Issuers() {


You can do

logger.Debug("issuers are [%s]", pp.Issuers())

I wanted to have them on separated lines, for clarify.

removed, I'll just print the public params.

aaadir · 2025-03-26T13:27:12Z

cmd/tokengen/main_test.go

 				"--issuers", "Error: failed to generate public parameters: failed to get issuer identity [aOrg1MSP]: invalid input [aOrg1MSP]",
 			},
-			ErrMsg: "Error: failed to generate public parameters: failed to get issuer identity [Error: failed to generate public parameters: failed to get issuer identity [aOrg1MSP]: invalid input [aOrg1MSP]]:",
+			ErrMsg: "Error: failed to generate public parameters: failed to setup issuer and auditors: failed to get issuer identity [Error: failed to generate public parameters: failed to get issuer identity [aOrg1MSP]: invalid input [aOrg1MSP]]: failed to load certificates from Error: failed to generate public parameters: failed to get issuer identity [aOrg1MSP]: invalid input [aOrg1MSP]/signcerts: stat Error: failed to generate public parameters: failed to get issuer identity [aOrg1MSP]: invalid input [aOrg1MSP]/signcerts: no such file or directory",


typo: "failed to load certificates from Error"

aaadir · 2025-03-26T13:49:48Z

integration/token/fungible/tests.go

 type OnRestartFunc = func(*integration.Infrastructure, string)

-func TestAll(network *integration.Infrastructure, auditorId string, onRestart OnRestartFunc, aries bool, sel *token3.ReplicaSelector) {
+func TestAll(network *integration.Infrastructure, auditorId string, onRestart OnRestartFunc, aries bool, orion bool, sel *token3.ReplicaSelector) {


consider, instead of "aries", "orion", and more possible future system-specific parameter names, maybe use a single generic parameter like network_name.

aaadir · 2025-03-26T13:53:12Z

integration/token/fungible/tests.go


 	SetKVSEntry(network, issuer, "auditor", auditor.Id())
 	CheckPublicParams(network, issuer, auditor, alice, bob, charlie, manager)
+	if orion {


consider, instead of mentioning a specific network_name in the code, maybe refer to its relevant trait. Then, either pass the trait as a parameter or maintain a structure with generic traits for every network type.

good point, I think we need to rework these tests. I'll open a github issue for it.

aaadir · 2025-03-26T14:04:40Z

token/core/fabtoken/v1/driver/driver.go

+	// log information about the public params
+	pp := publicParamsManager.PublicParams()
+	logger.Infof("new token driver for tms id [%s] with label and version [%s:%s]", tmsID, pp.Identifier(), pp.Version())
+	for _, issuer := range pp.Issuers() {


I see that this kind of debug-printing occurs often in the code.
Consider having a general debug-printing method for printing slices of strings - e.g. something like logger.logSliceOf("issuer", pp.Issuers())

alexandrosfilios · 2025-03-26T14:34:25Z

token/services/identity/membership/lm.go

+		l.logger.Debugf("identity [%s:%s] not in target identities", name, config.URL)
 	} else {
+		// give it high priority
+		priority = -1


We could have it as a constant or even better a type to explain whether a low or high number has higher priority

alexandrosfilios

LGTM modulo comments

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

adecaro added the improvements label Mar 23, 2025

adecaro self-assigned this Mar 23, 2025

adecaro linked an issue Mar 23, 2025 that may be closed by this pull request

dlog v1 finalzation: enforce at list one issuer #1009

Closed

adecaro changed the title ~~dlog v1 finalzation: enforce at list one issuer~~ dlog v1 finalzation: enforce at least one issuer Mar 24, 2025

adecaro changed the title ~~dlog v1 finalzation: enforce at least one issuer~~ dlog v1 finalization: enforce at least one issuer Mar 24, 2025

adecaro requested a review from alexandrosfilios March 25, 2025 07:00

adecaro force-pushed the f-1009 branch from 33ddd1f to 790aee7 Compare March 25, 2025 08:09

adecaro added 23 commits March 25, 2025 11:21

enforce at least one issuer

823719b

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

register issuer to the main TMS

8c4b97c

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

validate pp after having added issuers and auditors

8fc4230

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

register issues in topology

1cf57e4

update public params during test when needed LocalMembership should not discard identities not in the target list, just assign a low priority Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

disable some InfrastructureType to speedup the CI, temp fix

2cd1634

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

nwo: add all the specified issuers

a85ab91

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

add more public params checks

0cbe23c

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

register issuers in topologies

10f5f5a

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix TestPublicParamsUpdate, check for proper error

5dfc47c

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix test

18dc23e

support for fabtoken pp Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix PreparePublicParamsWithNewIssuer

179ab9e

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix fabtoken label

48184fe

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

different network name for orion

1ba3bf3

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix unit-tests and lint

c6dd5df

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

update pp on orion

1e4afbd

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix test

7e08c6d

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fabtoken T2 fix

7203196

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fetch and update view, for orion

57b1708

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

use the normalized tms id

9f0f4be

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix issue mixed topology

1d54c18

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

fix orion

afb66fa

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

cleanup

8cbe401

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

FetchAndUpdatePublicParams for all replicas

d74a12b

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

rebase fix

71c158f

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

adecaro force-pushed the f-1009 branch from 09dc5a6 to 71c158f Compare March 25, 2025 10:25

adecaro requested a review from aaadir March 25, 2025 13:34

alexandrosfilios reviewed Mar 26, 2025

View reviewed changes

aaadir reviewed Mar 26, 2025

View reviewed changes

alexandrosfilios reviewed Mar 26, 2025

View reviewed changes

alexandrosfilios approved these changes Mar 26, 2025

View reviewed changes

cleanup logs and priority mgmt

338471a

Signed-off-by: Angelo De Caro <angelo.decaro@gmail.com>

aaadir approved these changes Mar 27, 2025

View reviewed changes

adecaro merged commit d5d3a66 into main Mar 27, 2025
54 checks passed

adecaro deleted the f-1009 branch March 27, 2025 09:51

dlog v1 finalization: enforce at least one issuer #1010

dlog v1 finalization: enforce at least one issuer #1010

Uh oh!

Conversation

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

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!

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!

alexandrosfilios left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adecaro commented Mar 23, 2025 •

edited

Loading