Symbol - Transactions

[rg911]

No description provided.

[fboucquez]

When signing and cosign transactions, we should try to abstract the signing process. It can happen that the account doesn't know a private key (like Ledger and Trezor). In the old SDK, we assumed that all the accounts have the private key, and then the apps that use Ledger need to copy and paste core code for merging signatures and cosignatures (payload processing).

You can see the duplicated code:

https://github.com/symbol/symbol-sdk-typescript-javascript/blob/main/src/model/transaction/Transaction.ts#L214

https://github.com/symbol/desktop-wallet/blob/main/src/core/utils/Ledger.ts#L157
https://github.com/symbol/desktop-wallet/blob/main/src/core/utils/Ledger.ts#L186

Something inline of https://github.com/symbol/symbol-cli/blob/ledger_support/src/services/signing.service.ts#L44 probably.

Basically use a SigningAccount/Signer interface (instead of just the Account with private key) that knows how to sign/cosign a transaction. The private key account one would be the out-of-the-box signer.

[fboucquez]

I had a chat with @rg911 about how to represent the symbol transaction model in the SDK. We evaluated 2 alternatives:

1 - Create the user-friendly transaction hierarchy as we have in the legacy SDK.
2 - User to use the catbuffer builders directly + utility classes and helpers (signing, hashing, etc)

Both have pros and cons.

Transaction/State wrappers.

• It's easier to extend, you don't need to touch the generators. ✔️
• Can "absorb" some fixes from the end-user, One example could be the versioned voting key transactions ✔️
• Manual code to maintain, new transactions need to be manually added. ❌
• Any change in catbuffer required manually changing the objects. ❌
• All the ugly mappers. ❌
• It unifies objects coming from rest and catbuffer into the same hierarchy. Not an issue anymore/yet. ✔️

Catbuffer builders as-is

• Adding new transactions is easy, just adding a schema, you get the code for free. ✔️
• No extra hierarchy ✔️
• Harder to use for the end-user.:x:
• Catbuffer generator is harder to understand and maintain ❌
• But once fixed, it changes all the classes automatically. ✔️

We agreed to change the approach this time, the user would use the catbuffer builders to handle transactions (Option 2).

For this, I think we would need

1. Have plenty of examples on how to create transactions via builders and helpers. The examples to be part of the unit testing suite so we can do assertions on it (like hash and signature verification).
2. Improve the typescript catbuffer generator so the generated classes are easier to use. I have a few proposals that I think will make life easier for the end-user. Once we have the examples, we can compare the improvements and see what's better.

This idea can be applied to other states like accounts/metadata for merkle proofs.

What about NIS1? Is a NIS1 catbuffer and generator possible? If we add that to the catbuffer schemas, we may get all the builders for free.... Catbuffer schemas could be both symbol+nis1 schemas like we are unifying the SDKs.

I will be pushing a few PRs.

[rg911]

I had a chat with @rg911 about how to represent the symbol transaction model in the SDK. We evaluated 2 alternatives:

1 - Create the user-friendly transaction hierarchy as we have in the legacy SDK.
2 - User to use the catbuffer builders directly + utility classes and helpers (signing, hashing, etc)

Both have pros and cons.

Transaction/State wrappers.

• It's easier to extend, you don't need to touch the generators. ✔️
• Can "absorb" some fixes from the end-user, One example could be the versioned voting key transactions ✔️
• Manual code to maintain, new transactions need to be manually added. ❌
• Any change in catbuffer required manually changing the objects. ❌
• All the ugly mappers. ❌
• It unifies objects coming from rest and catbuffer into the same hierarchy. Not an issue anymore/yet. ✔️

Catbuffer builders as-is

• Adding new transactions is easy, just adding a schema, you get the code for free. ✔️
• No extra hierarchy ✔️
• Harder to use for the end-user.❌
• Catbuffer generator is harder to understand and maintain ❌
• But once fixed, it changes all the classes automatically. ✔️

We agreed to change the approach this time, the user would use the catbuffer builders to handle transactions (Option 2).

For this, I think we would need

1. Have plenty of examples on how to create transactions via builders and helpers. The examples to be part of the unit testing suite so we can do assertions on it (like hash and signature verification).
2. Improve the typescript catbuffer generator so the generated classes are easier to use. I have a few proposals that I think will make life easier for the end-user. Once we have the examples, we can compare the improvements and see what's better.

This idea can be applied to other states like accounts/metadata for merkle proofs.

What about NIS1? Is a NIS1 catbuffer and generator possible? If we add that to the catbuffer schemas, we may get all the builders for free.... Catbuffer schemas could be both symbol+nis1 schemas like we are unifying the SDKs.

I will be pushing a few PRs.

I would prefer using catbuffer builder directly, however with slight mods:

• we could try to start using native bigint in the buffer generator for ts.js
• consider using factory pattern makes the transaction building journey a bit more user-friendly?

[fboucquez]

we could try to start using native bigint in the buffer generator for ts.js

PR incoming :)

consider using factory pattern makes the transaction building journey a bit more user-friendly?

Unsure how that can look (factory of a builder?), the examples could give us an indication how useful they are. Ideally, any transaction related code should be generated so it's free for all the transactions. If the factory works, we can generate it from the schema.