Enhanced redeem with the signature of the issuer #1027

barvhaim · 2025-03-31T14:45:39Z

No description provided.

alexandrosfilios · 2025-04-02T01:08:04Z

Makefile

I think you will need a specific version here. the problem is that the dependency sometimes is updated because of other dependencies during a go mod. then you have to reset it back to that specific version needed.

This line stayed as-is and out of the scope of this PR

alexandrosfilios · 2025-04-02T01:08:15Z

integration/ports.go

@@ -60,8 +60,8 @@ var (

 	AllTestTypes = []*InfrastructureType{
 		WebSocketNoReplication,
-		LibP2PNoReplication,
-		WebSocketWithReplication,
+		//LibP2PNoReplication,


not to forget

barvhaim · 2025-04-08T12:08:10Z

@aaadir I implemented the fabotken + zkat logic, fixed the failing unit-tests and most of the integration tests.. what is left is integration tests related to "Auditor as an Issuer" cases

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

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

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

to pass the Issuer network identity. This allows the developer to avoid to call SetBinding Signed-off-by: Angelo De Caro <[email protected]> Signed-off-by: Allon Adir <[email protected]>

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

…hByID Signed-off-by: Allon Adir <[email protected]>

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

adecaro

LGTM overall modulo the requested changes. Thanks

adecaro · 2025-04-23T08:17:11Z

token/core/fabtoken/v1/actions/transfer_test.go

@@ -255,6 +255,7 @@ func TestTransferAction_Serialization(t *testing.T) {
 		Metadata: map[string][]byte{
 			"metadata": []byte("{\"foo\":\"bar\"}"),
 		},
+		Issuer: nil,


Please, a value different from nil to make sure the serialization works as expected.

The Issuer field should be nil for non-Redeem transactions.
For redeem it will be populated to the issuer's-PK inside the action.Serialize() that follows immediately after this line.
This is why the Redeem test also works for the ZK driver.

adecaro · 2025-04-23T08:17:38Z

token/core/fabtoken/v1/actions/transfer.go

@@ -164,6 +167,11 @@ func (t *TransferAction) GetMetadata() map[string][]byte {
 	return t.Metadata
 }

+// GetIssuer returns the issuer to sign the transaction
+func (t *TransferAction) GetIssuer() driver.Identity {


Please, add a unit-test for this function.

Added TestTransferAction_GetIssuer to token/core/fabtoken/v1/actions/transfer_test.go
done.

adecaro · 2025-04-23T08:18:08Z

token/core/fabtoken/v1/setup/setup_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"

 	"github.com/hyperledger-labs/fabric-token-sdk/token/driver"
+


remove this line

adecaro · 2025-04-23T08:19:38Z

token/core/fabtoken/v1/transfer.go

@@ -46,6 +46,7 @@ func NewTransferService(
 // Transfer returns a TransferAction as a function of the passed arguments
 // It also returns the corresponding TransferMetadata
 func (s *TransferService) Transfer(ctx context.Context, _ string, _ driver.OwnerWallet, tokenIDs []*token.ID, Outputs []*token.Token, opts *driver.TransferOptions) (driver.TransferAction, *driver.TransferMetadata, error) {
+	var isRedeem bool


please, move this declaration closer to where it is needed.

it's indeed needed much later, but it's set in the iteration over the outputs here below. But I moved the var declaration to right before this iteration.
done.

adecaro · 2025-04-23T08:19:46Z

token/core/fabtoken/v1/validator/validator_transfer.go

@@ -30,6 +30,7 @@ func TransferSignatureValidate(ctx *Context) error {
 		return errors.Errorf("invalid number of token inputs, expected at least 1")
 	}

+	var isRedeem bool


please, move this declaration closer to where it is needed.

adecaro · 2025-04-23T08:22:06Z

token/core/zkatdlog/nogh/v1/transfer/action.go

@@ -243,6 +247,11 @@ func (t *Action) GetMetadata() map[string][]byte {
 	return t.Metadata
 }

+// GetIssuer returns the issuer to sign the transaction
+func (t *Action) GetIssuer() driver.Identity {


please, add a unit-tests for this function.

adecaro · 2025-04-23T08:22:20Z

token/core/zkatdlog/nogh/v1/transfer/action.go

@@ -313,6 +322,13 @@ func (t *Action) Serialize() ([]byte, error) {
 		return nil, errors.Wrap(err, "failed to serialize outputs")
 	}

+	var issuer *pp.Identity


please, update TestAction_Validate to check also the added code.

TestAction_Validate doesn't really check the updated Serialize code, just runs action.Validate().
But I added an issuer to the action anyway, to see that Validate works.
Serialize is actually tested in another test here (TestSerialization) so I also updated it (by creating an issuer in randomAction).
done.

adecaro · 2025-04-23T08:23:05Z

token/core/zkatdlog/nogh/v1/validator/validator_transfer.go

+	}
+
+	if isRedeem {
+		ctx.Logger.Infof("action is a redeem, verify the signature of the issuer")


please, use Debugf here

done.
(what is the general rule for when to use Infof vs. Debugf?)

adecaro · 2025-04-23T08:24:09Z

token/driver/request.go

@@ -373,6 +373,8 @@ type TransferMetadata struct {
 	// ExtraSigners is the list of extra identities that are not part of the transfer action per se
 	// but needs to sign the request
 	ExtraSigners []Identity
+	// Issuer contains the identity of the issuer to sign the transfer action
+	Issuer Identity


please, update TestTokenRequestMetadataSerialization, to include also this field

adecaro · 2025-04-23T08:25:02Z

token/services/ttx/transfer_opts.go

+	IssuerFSCIdentityKey = "IssuerFSCIdentityKey"
+)
+
+func WithFSCIssuerIdentity(issuerFSCIdentity view.Identity) token.TransferOption {


please, add comments to these two functions

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

adecaro

LGTM

alexandrosfilios reviewed Apr 2, 2025

View reviewed changes

aaadir force-pushed the f-904-issuer-action branch from a82122f to c8be372 Compare April 17, 2025 03:55

adecaro force-pushed the f-904-issuer-action branch from c8be372 to ae792c2 Compare April 17, 2025 04:39

aaadir force-pushed the f-904-issuer-action branch 2 times, most recently from 52176fb to 967ed7e Compare April 22, 2025 09:55

aaadir changed the title ~~Redeem: issuer in action for Fabtoken WIP~~ Enhanced redeem with the signature of the issuer Apr 23, 2025

barvhaim and others added 22 commits April 23, 2025 10:30

redeem: fabtoken issuer in action

54bed5a

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: fabtoken issuer in action

39a9544

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: fabtoken issuer in action

3b706b2

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: issuer in transfer metadata proto

430c0bc

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: added issuer in requestsToBeSigned

12db77c

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: validator logic

1a93365

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: validator logic

c5c6647

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: zkat logic

89fb9c5

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: rebase

37256b1

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: fabtoken issuer in action

11e4294

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

redeem: fabtoken issuer in action

31a0ced

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

redeem: issuer in transfer metadata proto

b94c272

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: validator logic

bb0e2f3

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: validator logic

8916706

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: zkat logic

56bac0d

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: make checks

255c150

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: unit tests

db59aba

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: unit tests

285b679

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

redeem: integration tests

5ccc5f3

Signed-off-by: bar haim <[email protected]> Signed-off-by: Allon Adir <[email protected]>

introduce WithFSCIssuerIdentity as a way for the developer

a36b935

to pass the Issuer network identity. This allows the developer to avoid to call SetBinding Signed-off-by: Angelo De Caro <[email protected]> Signed-off-by: Allon Adir <[email protected]>

WithFSCIssuerIdentity: fix bug (allocate map prior to use)

a07c8d2

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

WithFSCIssuerIdentity: remove redundant SetBindings and fix RedeemCas…

cf0c26a

…hByID Signed-off-by: Allon Adir <[email protected]>

aaadir added 3 commits April 23, 2025 10:30

feat: Redeem with issuer signer

f97e874

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

feat: Redeem with issuer signer

1404fe7

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

feat: Redeem with SetBiding

9aa7a05

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

aaadir force-pushed the f-904-issuer-action branch from d30b1d7 to 9aa7a05 Compare April 23, 2025 07:32

adecaro self-assigned this Apr 23, 2025

adecaro added the enhancement New feature or request label Apr 23, 2025

adecaro marked this pull request as ready for review April 23, 2025 08:08

adecaro self-requested a review April 23, 2025 08:08

adecaro requested changes Apr 23, 2025

View reviewed changes

feat: Redeem unit testing

1f6ec99

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

This was linked to issues Apr 24, 2025

enhanced redeem with the signature of the issuer #890

Open

Enhanced redeem with the signature of the issuer: basic operation #1059

Closed

aaadir mentioned this pull request Apr 24, 2025

enhanced redeem with the signature of the issuer #890

Open

aaadir removed a link to an issue Apr 24, 2025

enhanced redeem with the signature of the issuer #890

Open

aaadir mentioned this pull request Apr 24, 2025

enhanced redeem: add option to specify the issuer #905

Open

adecaro self-requested a review April 24, 2025 11:01

adecaro approved these changes Apr 24, 2025

View reviewed changes

aaadir merged commit f28fe70 into main Apr 24, 2025
55 checks passed

aaadir deleted the f-904-issuer-action branch April 24, 2025 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced redeem with the signature of the issuer #1027

Enhanced redeem with the signature of the issuer #1027

barvhaim commented Mar 31, 2025

alexandrosfilios Apr 2, 2025

barvhaim Apr 7, 2025

alexandrosfilios Apr 2, 2025

aaadir Apr 23, 2025

barvhaim commented Apr 8, 2025

adecaro left a comment

adecaro Apr 23, 2025

aaadir Apr 23, 2025 •

edited

Loading

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025 •

edited

Loading

adecaro Apr 23, 2025

aaadir Apr 24, 2025

adecaro Apr 23, 2025

aaadir Apr 24, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro Apr 23, 2025

aaadir Apr 23, 2025

adecaro left a comment

		@@ -10,6 +10,7 @@ import (
		"testing"

		"github.com/hyperledger-labs/fabric-token-sdk/token/driver"

Enhanced redeem with the signature of the issuer #1027

Enhanced redeem with the signature of the issuer #1027

Conversation

barvhaim commented Mar 31, 2025

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

barvhaim commented Apr 8, 2025

adecaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaadir Apr 23, 2025 • 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

Choose a reason for hiding this comment

aaadir Apr 23, 2025 • 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

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

adecaro left a comment

Choose a reason for hiding this comment

aaadir Apr 23, 2025 •

edited

Loading

aaadir Apr 23, 2025 •

edited

Loading