Skip to content
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.

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

💡 [BREAKING CHANGES] Get Account :: Utility functions #998

Closed
darrenvechain opened this issue Jun 19, 2024 · 5 comments · Fixed by #1364
Closed

💡 [BREAKING CHANGES] Get Account :: Utility functions #998

darrenvechain opened this issue Jun 19, 2024 · 5 comments · Fixed by #1364

Comments

@darrenvechain
Copy link
Member

darrenvechain commented Jun 19, 2024

Summary

It would be nice if getAccount returned a class or an object with some utility functions built in so it is easier to debug and read account balances etc.

Current code:

// Returns {balance, energy, hasCode}
const account = await client.accounts.getAccount(account)

Basic Example

It would be nice to have some extra functions such as:

const account = await client.accounts.getAccount(account)
console.log(account.parsedVet()) //if my VET balance is `0x43c33c1937564800000`, then it prints `20000`
@claytonneal
Copy link
Member

Its unclear to me why we return hex strings for getAccount for the VET & VTHO balance
perhaps to consider to change the AccountDetail interface so that balance and energy are of type BigNumber
thanks

@lucanicoladebiasi
Copy link
Contributor

Since I reworked che address introducing a full fledged class 'Address' extending 'HEX', I suggest I. take the responsibility of such task implementing what missing and required.

@fabiorigam
Copy link
Member

No problem Luca

@victhorbi victhorbi changed the title 💡 Get Account :: Utility functions 💡 [BREAKING CHANGES] Get Account :: Utility functions Jul 19, 2024
@victhorbi victhorbi added this to the RC milestone Jul 19, 2024
@nwbrettski
Copy link
Collaborator

@lucanicoladebiasi has implemented using the OO approach. Will showcase as a demo during the refinement session.

@lucanicoladebiasi
Copy link
Contributor

lucanicoladebiasi commented Sep 30, 2024

The solution I propose is

to rename the interface AccountDetail in AccountData in packages/network/src/thor-client/accounts/types.d.ts. The interface is needed as container of data to cast the unknown object from

(await this.thor.httpClient.http(
                'GET',
                thorest.accounts.get.ACCOUNT_DETAIL(address),
                {
                    query: buildQuery({ revision: options?.revision })
                }
            )) as AccountData

in the method getAccount of the class AccountsModule at packages/network/src/thor-client/accounts/accounts-module.ts

Renaming AccountDetail to AccountData free the name to be used to implement the AccountDetail class in the file packages/network/src/thor-client/accounts/accounts-module.ts.
The class implements AccountData hence the solution preserves back compatibility.
The class exposes the computed properties

     /**
     * Returns the balance of the account in {@link VET}.
     */
    get vet(): VET {
        return VET.of(Units.formatEther(FixedPointNumber.of(this.balance)));
    }

    /**
     * Returns the energy balance of the account in {@link VTHO}.
     */
    get vtho(): VTHO {
        return VTHO.of(Units.formatEther(FixedPointNumber.of(this.energy)));
    }

returning objects of the VeChain Data Model type.

See at packages/network/tests/thor-client/accounts/accounts.solo.test.ts to see the proposed solution in action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants