asset transfer #639

adecaro · 2024-05-27T10:39:58Z

This PR adapts the code in #437 to the current shape of the token-sdk. The code is mostly contribution of @HagarMeir .

To be addressed in another PR:

Unit tests
Resolve TODO
Additional integration tests

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

alexandrosfilios · 2024-05-27T15:40:08Z

integration/token/interop/tests.go

+
+func TestAssetTransferWithTwoNetworks(network *integration.Infrastructure) {
+	// give some time to the nodes to get the public parameters
+	time.Sleep(10 * time.Second)


Maybe this sleep is better after start

we have it in all tests, maybe we can replace it with a different check in another PR.

alexandrosfilios · 2024-05-27T15:53:12Z

integration/token/interop/views/pledge/fast_pledge.go

+	assert.Equal(me, pledgeInfo.Script.Recipient, "recipient is different [%s]!=[%s]", me, pledgeInfo.Script.Recipient)
+
+	// Store the pledge and send a notification back
+	_, err = context.RunView(pledge.NewAcceptPledgeIndoView(pledgeInfo))


what's this?

@adecaro There is a typo in the name of the method.
NewAcceptPledgeInfoView instead of NewAcceptPledgeIndoView

heheheh, good catch.

alexandrosfilios · 2024-11-18T09:05:50Z

integration/token/interop/dlog/dlog_test.go

@@ -23,7 +24,7 @@ import (
 var _ = Describe("DLog end to end", func() {
 	BeforeEach(func() { token.Drivers = append(token.Drivers, "dlog") })

-	for _, t := range integration2.AllTestTypes {
+	for _, t := range integration2.WebSocketNoReplicationOnly {


We need to revert this, right?

alexandrosfilios · 2024-11-18T09:36:03Z

integration/token/interop/views/auditor.go

+		switch {
+		case output.IsHTLC():
+			// check script details
+			script, err := output.HTLC()


maybe we could use just one interface for both, like

Script() ValidatableScript

that only contains the method validate. That would be maybe more extensible

this is just an example, one might perform more checks...

alexandrosfilios · 2024-11-18T09:38:26Z

integration/token/interop/views/pledge/claim.go

+	"github.com/hyperledger-labs/fabric-token-sdk/token/token"
+)
+
+// Claim contains the input information to claim a token


Maybe to give more context

claim a token that has already been pledged, but not yet claimed by someone else, reclaimed by the owner or redeemed by the issuer of the network.

added the comment to the View

alexandrosfilios · 2024-11-18T10:04:08Z

integration/token/interop/views/pledge/fast_pledge.go

+	assert.NoError(err, "failed to request claim")
+
+	return view2.AsResponder(context, session,
+		func(context view.Context) (interface{}, error) {


We could extract this to a view and re-use the logic in both pledge and fast pledge

this is just an example in an integration test.

alexandrosfilios · 2024-11-18T10:48:31Z

token/core/common/interop/pledge/metadata.go

+}
+
+func IssueActionMetadata(attributes map[string][]byte, opts *driver.IssueOptions) (map[string][]byte, error) {
+	var metadata *IssueMetadata


Maybe to avoid nesting

if len(opts.Attributes) == 0 { return nil, nil } tokenID, hasTokenID := ... network, hasNetwork := ... if !hasTokenID || !hasNetwork { return nil, nil } marshalled, err := json.Marshal(&IssueMetadata{tokenID.(*token.ID), network.(string)}) if err != nil {return nil, err} key := common.Hashable(marshalled).String() attributes := map[string][]byte{ key: marshalled } if proofOpt, ok := ...; ok { attributes[key + ProofOfClaimSuffix] = proofOpt.([]byte) } return attributes, nil

alexandrosfilios · 2024-11-18T10:49:02Z

token/core/common/interop/pledge/metadata.go

+	OriginNetwork string
+}
+
+func IssueActionMetadata(attributes map[string][]byte, opts *driver.IssueOptions) (map[string][]byte, error) {


Since we don't use the attributes as an input, it's better to avoid passing it as a param for more clarity. Otherwise this implies that the attributes we output depend on the input attributes. Which would require that the different metadataGenerators be called in a specific order.

attributes is used an input and output, you can have multiple functions manipulating these attributes

alexandrosfilios · 2024-11-18T10:51:51Z

token/core/common/interop/pledge/metadata.go

+	var metadata *IssueMetadata
+	var proof []byte
+	if len(opts.Attributes) != 0 {
+		tokenID, ok1 := opts.Attributes["github.com/hyperledger-labs/fabric-token-sdk/token/services/interop/pledge/tokenID"]


We could have these attributes as constants

alexandrosfilios · 2024-11-18T12:00:54Z

token/core/common/interop/pledge/validator_pledge.go

+				return errors.Wrap(err, "failed constructing metadata key")
+			}
+
+			if out.IsRedeem() {


It would make it a bit more clear to re-use the common parts:

if out.IsRedeem() { key = pledge.RedeemPledgeKey + key } else { if !script.Sender.Equal(...) {...} key = pledge.MetadataReclaimKey + key } v, ok := ctx.TransferAction.GetMetadta()[key] ...

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

KElkhiyaoui · 2024-11-21T17:01:44Z

integration/token/interop/views/pledge/redeem.go

+	assert.NoError(err, "failed to retrieve pledged token during redeem")
+
+	// make sure token was in fact claimed in the other network
+	proof, err := pledge.RequestProofOfTokenWithMetadataExistence(context, rv.TokenID, rv.TMSID, script)


Maybe we change the name of the method to RequestProofOfExistenceOfTokenWithMetadata?

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

KElkhiyaoui · 2024-11-22T08:47:45Z

token/core/common/interop/htlc/validator_htlc.go

+		if owner.Type == htlc.ScriptType {
+			// Then, the first output must be compatible with this input.
+			if len(ctx.TransferAction.GetOutputs()) != 1 {
+				return errors.New("invalid transfer action: an htlc script only transfers the ownership of a token")


Maybe we should also enforce that len(Inputs) == 1 since a claim and a reclaim are just ownership transfers.

KElkhiyaoui · 2024-11-22T09:47:35Z

token/core/common/interop/pledge/metadata.go

+
+	tokenID, hasTokenID := opts.Attributes[TokenIDKey]
+	network, hasNetwork := opts.Attributes[NetworkKey]
+	if !hasTokenID && !hasNetwork {


Why do we perform this check? It does not seem to have an impact on the output of the method.

KElkhiyaoui · 2024-11-22T09:52:59Z

token/core/common/interop/pledge/validator_pledge.go

+				return err
+			}
+			if time.Now().Before(script.Deadline) {
+				return errors.New("cannot reclaim pledge yet: wait for timeout to elapse.")


It's both reclaim and redeem. Shall we say cannot change the state of the pledge yet?

KElkhiyaoui · 2024-11-22T10:10:32Z

token/services/interop/pledge/wallet.go

+	logger.Debugf("Is Mine [%s,%s,%s]? [%b] with [%s]", view2.Identity(tok.Owner.Raw), tok.Type, tok.Quantity, len(ids) != 0, ids)
+	return "", ids, len(ids) != 0
+}
+


What does Issued implement. A comment will be helpful.

KElkhiyaoui · 2024-11-22T10:11:38Z

token/core/common/interop/pledge/validator_pledge.go

+			v, ok := ctx.TransferAction.GetMetadata()[pledge.MetadataKey+script.ID]
+			if !ok {
+				return errors.Errorf("empty metadata for pledge script with identifier %s", script.ID)
+			}


What information does this metadata convey?

Going through the rest of the code, this seems to enforce to enforce the uniqueness of PledgeID.

KElkhiyaoui · 2024-11-22T10:12:52Z

token/core/fabtoken/actions.go

+}
+
+func (t *Output) GetOwner() []byte {
+	if t.Owner != nil {


Should this be if ! t.IsRedeem()?

KElkhiyaoui · 2024-11-22T10:14:37Z

token/services/interop/state/fabric/core/state.go

+	}, nil
+}
+
+func (p *StateQueryExecutor) Exist(tokenID *token.ID) ([]byte, error) {


KElkhiyaoui · 2024-11-22T10:19:51Z

token/services/interop/pledge/stream.go

+			logger.Debugf("failed unmarshalling pledge script [%s]: [%s]", tok, err)
+			return nil
+		}
+		if script.Sender.IsNone() || script.Recipient.IsNone() {


Should we return an error here?

KElkhiyaoui · 2024-11-22T10:25:24Z

token/services/interop/pledge/scanner.go

+
+// IDExists scans the ledger for a pledge identifier, taking into account the timeout
+// IDExists returns true, if entry identified by key (MetadataKey+pledgeID) is occupied.
+func IDExists(ctx view.Context, pledgeID string, timeout time.Duration, opts ...token.ServiceOption) (bool, error) {


This method does not seem to be used elsewhere.

KElkhiyaoui · 2024-11-22T10:28:59Z

token/services/interop/pledge/endorsement.go

+	return &ReceiveTransactionView{}
+}
+
+func (f *ReceiveTransactionView) Call(context view.Context) (interface{}, error) {


Shall we use v or rtv instead of f?

KElkhiyaoui · 2024-11-22T11:24:03Z

token/core/zkatdlog/crypto/audit/auditor.go

+	if err != nil {
+		return errors.Errorf("input owner at index [%d] cannot be unmarshalled", index)
+	}
+	scriptInf := &htlc.ScriptInfo{}


Shall move ScriptInfo under a common repo. Since this is a bit confusing.

KElkhiyaoui · 2024-11-22T11:33:35Z

token/core/zkatdlog/crypto/audit/auditor.go

+	if err := json.Unmarshal(token.Owner.OwnerInfo, scriptInf); err != nil {
+		return errors.Wrapf(err, "failed to unmarshal script info")
+	}
+	scriptSender, _, scriptIssuer, err := pledge.GetScriptSenderAndRecipient(owner)


Shall we call the method GetScriptStakeholders?

KElkhiyaoui · 2024-11-22T12:52:41Z

token/services/interop/pledge/recipients.go

+		if err := session.Receive(recipientData); err != nil {
+			return nil, errors.Wrapf(err, "failed to receive recipient data")
+		}
+		//if err := tms.WalletManager().RegisterRecipientIdentity(recipientData); err != nil {


Shall we remove the commented code? Or is this a todo?

KElkhiyaoui · 2024-11-22T13:09:11Z

token/services/interop/pledge/claim.go

+	}
+	logger.Debugf("proof of existence in claim request is valid [%s]", err)
+
+	if !req.ClaimDeadline.After(time.Now()) {


Shall we start with checking the time first?

KElkhiyaoui · 2024-11-22T13:13:57Z

token/services/interop/pledge/distribute.go

+		return nil, errors.Wrapf(err, "failed storing pledge info")
+	}
+
+	// raw, err := a.info.Bytes()


Can we remove this?

KElkhiyaoui · 2024-11-22T13:18:59Z

token/services/interop/pledge/state.go

+	if err != nil {
+		return nil, errors.WithMessagef(err, "failed getting prover for [%s]", c.source)
+	}
+	return p.Exist(c.tokenID)


KElkhiyaoui · 2024-11-22T13:48:11Z

token/services/interop/state/fabric/core/state.go

+	}, nil
+}
+
+func (v *StateVerifier) VerifyProofExistence(proofRaw []byte, tokenID *token.ID, metadata []byte) error {


How about VerifyProofOfTokenExistence?

KElkhiyaoui · 2024-11-22T13:49:38Z

token/services/interop/state/fabric/core/state.go

+	return nil
+}
+
+func (v *StateVerifier) VerifyProofNonExistence(proofRaw []byte, tokenID *token.ID, origin string, deadline time.Time) error {


How about VerifyProofOfTokenNonExistence

KElkhiyaoui · 2024-11-22T13:56:42Z

token/services/network/common/rws/keys/keys.go

@@ -88,6 +94,25 @@ func (t *Translator) CreateTransferActionMetadataKey(key string) (translator.Key
 	return createCompositeKey(TransferActionMetadataPrefix, []string{key})
 }

+func (t *Translator) CreateProofOfExistenceKey(tokenId *token.ID) (string, error) {


Maybe CreateProofOfTokenExistenceKey

KElkhiyaoui · 2024-11-22T13:57:16Z

token/services/network/common/rws/keys/keys.go

+	})
+}
+
+func (t *Translator) CreateProofOfMetadataExistenceKey(tokenID *token.ID, origin string) (string, error) {


Maybe CreateProofOfTokenMetadataExistenceKey

KElkhiyaoui · 2024-11-22T13:58:22Z

token/services/network/common/rws/keys/keys.go

+	return createCompositeKey(ProofOfExistencePrefix, []string{id})
+}
+
+func (t *Translator) CreateProofOfNonExistenceKey(tokenID *token.ID, origin string) (string, error) {


Maybe CreateProofOfTokenMetadataNonExistenceKey

adecaro force-pushed the f-447-bis branch 3 times, most recently from 33a1ca7 to 13f5dd3 Compare May 27, 2024 13:02

adecaro requested a review from alexandrosfilios May 27, 2024 13:44

adecaro mentioned this pull request May 27, 2024

asset transfer #447 #437

Closed

adecaro linked an issue May 27, 2024 that may be closed by this pull request

Introduce asset transfer between two fabric networks #447

Open

adecaro self-assigned this May 27, 2024

adecaro added the improvements label May 27, 2024

adecaro force-pushed the f-447-bis branch 2 times, most recently from bd56a9f to 6040977 Compare June 1, 2024 05:13

adecaro force-pushed the f-447-bis branch 2 times, most recently from 321360b to f9d2870 Compare November 12, 2024 14:22

adecaro requested a review from KElkhiyaoui November 16, 2024 07:02

adecaro force-pushed the f-447-bis branch from 4305ba0 to 0483a94 Compare November 18, 2024 08:01

adecaro added 13 commits November 18, 2024 11:11

asset transfer

5f12c45

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

rebase fix

d73b496

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

update

5286cf3

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

interop.mk, restore entry

a0c45a9

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

cleanup

57b215e

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

info to debug

9626842

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

topology fix

43a5619

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

sdk fix

3b701b3

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

move htlc and pledge validator to common

7047ec6

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

additional refactoring

4147f14

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

cleanup

e3200f6

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

minor fix

ac77efb

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

complete rebase

78baa5f

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

adecaro force-pushed the f-447-bis branch from 0f612fb to 78baa5f Compare November 18, 2024 11:40

alexandrosfilios reviewed Nov 18, 2024

View reviewed changes

fix transfer action inputs

e99721b

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

adecaro added 2 commits November 19, 2024 06:27

adding labels to tests

fde6e00

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

go mod tidy up

973bc9f

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

adecaro force-pushed the f-447-bis branch from 3967a85 to 973bc9f Compare November 20, 2024 06:07

adecaro added 2 commits November 20, 2024 07:08

asset transfer only on websocket with replicas

17cea6a

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

cleanup for the comments

63b1996

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

adecaro requested a review from alexandrosfilios November 20, 2024 06:31

unit test fix

77696f4

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

KElkhiyaoui reviewed Nov 21, 2024

View reviewed changes

method refactoring

d897d92

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

KElkhiyaoui reviewed Nov 22, 2024

View reviewed changes

asset transfer #639

Are you sure you want to change the base?

asset transfer #639

Conversation

adecaro commented May 27, 2024 • 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

KElkhiyaoui Nov 18, 2024 • 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

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

KElkhiyaoui Nov 22, 2024 • 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

KElkhiyaoui Nov 22, 2024 • 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

KElkhiyaoui Nov 22, 2024 • 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

adecaro commented May 27, 2024 •

edited

Loading

KElkhiyaoui Nov 18, 2024 •

edited

Loading

KElkhiyaoui Nov 22, 2024 •

edited

Loading

KElkhiyaoui Nov 22, 2024 •

edited

Loading

KElkhiyaoui Nov 22, 2024 •

edited

Loading