Skip to content

feat: token version #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: token version #261

wants to merge 2 commits into from

Conversation

raul-oliveira
Copy link

Motivation

This PR is part of the Dynamic transaction model implementation.

According to the docs, tokens can be created with two versions:

Token info version

Given Hathor's Anatomy of a Transaction RFC, it is reasonable to suggest that the byte used by the token creation transaction token_info_version will be used to determine fee-created tokens.

Since each custom token id is the hash of the token creation transaction that created it, we can assume the enum values below can be assigned to the token_info_version byte in the token creation tx and then we can retrieve it.

So, by adding a TokenInfoVersion enum we have:

DEPOSIT = 1 (as is)
FEE = 2

The wallet-service is used by wallet-lib, a project that expect the version field to be available on the getTokenDetails API.

Acceptance Criteria

  • Add the version column in the database with a migration
  • Update the TokenInfo entity to handle the version
    • Add to sync-daemon workspace
    • Add to wallet-service workspace
  • Update some test utils functions to handle the version field
  • Update the sql statements that brings the token details
  • Add the version field in the getTokenInformation API method.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged
  • Make sure either the unit tests and/or the QA tests are capable of testing the new features
  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@raul-oliveira raul-oliveira self-assigned this May 30, 2025
@raul-oliveira raul-oliveira moved this from Todo to In Progress (Done) in Hathor Network May 30, 2025
andreabadesso
andreabadesso reviewed Jun 3, 2025
View reviewed changes
README.md

#### Clone the project and install dependencies

`git clone https://github.com/HathorNetwork/hathor-wallet-service-sync_daemon.git`
```sh
$ git clone https://github.com/HathorNetwork/hathor-wallet-service-sync_daemon.git
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace this with

git@github.com:HathorNetwork/hathor-wallet-service.git?

README.md
@@ -54,6 +79,16 @@ AWS_SECRET_ACCESS_KEY="..."

These are used for communicating with the alert SQS

#### Docker images

Some packages depends on some docker images. To build them you'll need to have Hathor VPN access configured, check this [link](https://github.com/HathorNetwork/ops-tools/blob/master/terraform/wireguard-vpn/SOP.md#adding-a-new-client-to-the-vpn) for it.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which ones?

packages/wallet-service/tsconfig.json
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"composite": true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

packages/daemon/src/types/token.ts
@@ -5,9 +5,26 @@
* LICENSE file in the root directory of this source tree.
*/

import { constants } from '@hathor/wallet-lib';
import { constants } from "@hathor/wallet-lib";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

packages/wallet-service/src/types.ts
Comment on lines +167 to +183
export enum TokenInfoVersion {
DEPOSIT = 1,
FEE = 2
}

export interface ITokenInfo {
id: string;
name: string;
symbol: string;
version?: TokenInfoVersion | null;
}

export interface ITokenInfoOptions extends ITokenInfo {
transactions?: number
}

export class TokenInfo implements ITokenInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have those types in the wallet-lib?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm introducing them in a new PR that should be merged before this.
The types we have there: TokenInfoVersion, ITokenInfo (With a different name)

r4mmer
r4mmer reviewed Jun 9, 2025
View reviewed changes
package.json
"node": ">=18"
"node": ">=20"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should upgrade to 22, since the current lib version already requires node 22

packages/daemon/src/types/db.ts
@@ -119,6 +119,7 @@ export interface TokenInformationRow extends RowDataPacket {
name: string;
symbol: string;
transactions: number;
version?: number | null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why null?

db/migrations/20250529233113-add-token-version.js
async up(queryInterface, Sequelize) {
queryInterface.addColumn("token", "version", {
type: Sequelize.INTEGER,
allowNull: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why allow null?
We could create the column with a default value of 0 (or whatever value the tokenVersion.DEPOSIT is) so that all "current listed" tokens will be marked as DBTs (Deposit Based Token).

What do you think?

@raul-oliveira raul-oliveira moved this from In Progress (Done) to In Progress (WIP) in Hathor Network Jun 12, 2025
@raul-oliveira raul-oliveira moved this from In Progress (WIP) to Todo in Hathor Network Jul 16, 2025
@raul-oliveira raul-oliveira moved this from Todo to In Progress (WIP) in Hathor Network Jul 16, 2025
@raul-oliveira raul-oliveira moved this from In Progress (WIP) to Todo in Hathor Network Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreabadesso andreabadesso andreabadesso left review comments

@r4mmer r4mmer r4mmer left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

@raul-oliveira raul-oliveira

Labels
None yet
Projects
Hathor Network
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raul-oliveira @andreabadesso @r4mmer