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

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

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.

Is it possible to have multiple inputs that point to the same output owner?

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()?

or IsRedeem can be equivalently return len(t.GetOwner()) == 0

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

integration/token/common/sdk/fall/sdk.go

alexandrosfilios · 2024-11-21T06:46:48Z

integration/token/common/sdk/fall/sdk.go

+
+	if fabricEnabled {
+		return errors.Join(
+			digutils.Register[fabric2.RelayProvider](p.Container()),


Do all of the apps that use the fdlog sdk need the relay and pledge services? Otherwise maybe itd make more sense to extend to another sdk

alexandrosfilios · 2024-11-21T06:49:01Z

integration/token/interop/fabtoken/fabtoken_test.go

+			ts, selector := newTestSuiteInteropAssetTransfer(t.CommType, t.ReplicationFactor, "alice", "bob")
+			AfterEach(ts.TearDown)
+			BeforeEach(ts.Setup)
+			It("Performed a cross network asset transfer", Label("T7"), func() { interop.TestAssetTransferWithTwoNetworks(ts.II, selector) })


Until we figure out a solution, maybe we could run both legs of this test as T7-websocket and T7-libp2p with a todo

alexandrosfilios · 2024-11-25T10:16:17Z

token/services/interop/pledge/state.go

+	destination string
+}
+
+func NewCollectProofOfTokenWithMetadataExistenceView(tokenID *token2.ID, origin string, destination string) *CollectProofOfTokenWithMetadataExistenceView {


Maybe it would be handy to use specific types for origin and destination like NetworkURL or NamespaceURL, because it is a bit ambiguous

alexandrosfilios · 2024-11-25T11:05:13Z

token/services/interop/pledge/store.go

+	}
+
+	var res *Info
+	for it.HasNext() {


What would the disadvantage be if we used a db? Or if we added tokenID as part of the compositeID

alexandrosfilios · 2024-11-26T08:14:06Z

token/services/interop/pledge/wallet.go

+)
+
+func MyOwnerWallet(sp view.ServiceProvider) (*token.OwnerWallet, error) {
+	w := token.GetManagementService(sp).WalletManager().OwnerWallet("")


return GetOwnerWallet(sp, "")

alexandrosfilios · 2024-11-26T08:18:03Z

token/services/interop/pledge/wallet.go

+	var res []*token2.UnspentToken
+	var scripts []*Script
+	for _, tok := range unspentTokens.Tokens {
+		if tok.Id.String() == tokenID.String() {


We cannot have multiple tokens with the same id. So I would expose another method from the query service GetUnspentToken(id *token2.ID, typ string) (*token2.UnspentToken, error). As an interim solution we can have a decorator that just iterates and finds it (though I think a DB query is equally fast to implement).. In any case I would return at the end of this if.

alexandrosfilios · 2024-11-26T08:22:07Z

token/services/interop/pledge/wallet.go

+	ListUnspentTokens() (*token2.UnspentTokens, error)
+}
+
+type IssuerWallet struct {


The functionality is the same for OwnerWallet. I would create one type wallet and then

type IssuerWallet wallet type OwnerWallet wallet

alexandrosfilios · 2024-11-26T08:24:07Z

token/services/interop/pledge/wallet.go

+	for _, beneficiary := range []struct {
+		identity view2.Identity
+		desc     string
+		prefix   string


We don't need prefix since it is only used in this method and it can be constructed by desc.

alexandrosfilios · 2024-11-26T08:26:25Z

token/services/interop/pledge/wallet.go

+	} {
+		logger.Debugf("Is Mine [%s,%s,%s] as a %s?", view2.Identity(tok.Owner.Raw), tok.Type, tok.Quantity, beneficiary.desc)
+		// TODO: differentiate better
+		if wallet, err := s.WalletService.OwnerWallet(beneficiary.identity); err == nil {


We could extend this method to take more identities in the input to avoid redundant locks in the future.

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 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

alexandrosfilios requested changes Nov 26, 2024

View reviewed changes

adecaro marked this pull request as draft November 26, 2024 14:17

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

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

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

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 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