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

RING balance on HydraDX does not show correctly. #46

Closed
hackfisher opened this issue Sep 30, 2024 · 23 comments
Closed

RING balance on HydraDX does not show correctly. #46

hackfisher opened this issue Sep 30, 2024 · 23 comments
Assignees
Labels
bug Indicates a defect or error in the code that needs to be fixed. high priority Tasks that require immediate attention and should be addressed as soon as possible.

Comments

@hackfisher
Copy link
Contributor

a

b

Please leave an address and let us know if you need some RING tokens for testing purpose.

@hackfisher
Copy link
Contributor Author

Discussed in https://github.com/orgs/ringecosystem/discussions/5

Originally posted by boundless-forest September 30, 2024
From community member:

image


image
image

@hamidroohi92 @jiguantong Please take a look.

@jiguantong
Copy link

jiguantong commented Sep 30, 2024

The front end got the wrong token.
image

Should get RING balance from tokens pallet.

image

@boundless-forest boundless-forest added bug Indicates a defect or error in the code that needs to be fixed. high priority Tasks that require immediate attention and should be addressed as soon as possible. labels Sep 30, 2024
@hamidroohi92
Copy link
Collaborator

let me check and give you the result

@hackfisher
Copy link
Contributor Author

ChatGPT

To get the balance of a specific token using the Polkadot.js API from a token pallet, follow these steps:

Prerequisites:

  • Ensure you have Polkadot.js installed and set up.
  • Access to the network where the token pallet is deployed (e.g., a Substrate-based chain).

Steps:

  1. Import the API:
    First, you need to initialize and connect to the network using Polkadot.js API.

    import { ApiPromise, WsProvider } from '@polkadot/api';
    
    // Connect to a Substrate-based node (update the endpoint as needed)
    const provider = new WsProvider('wss://your-node-endpoint');
    const api = await ApiPromise.create({ provider });
  2. Specify the Token and Account:
    The token pallet often uses the assetId to identify different tokens. You need to know the specific asset ID of the token and the account address whose balance you want to fetch.

    const assetId = 1;  // Example asset ID
    const account = '5Gw3s7q4QLkSWwknsiKTUzSi9Mp6P6wT8YrdJtMgnRr4A51';  // Example account address
  3. Query the Token Balance:
    Depending on the pallet used (e.g., assets or tokens), the query can vary. Here's an example assuming you are working with the assets pallet:

    const balance = await api.query.assets.account(assetId, account);
    console.log(`Balance: ${balance.balance}`);

    If you're using the tokens pallet (common in chains like Statemint), you might do something like this:

    const balance = await api.query.tokens.accounts(account, assetId);
    console.log(`Free balance: ${balance.free}`);

Notes:

  • The assetId identifies the specific token in the multi-token environment.
  • The api.query.tokens.accounts and api.query.assets.account are used to fetch balance information for an account in the context of a specific token.

This approach provides the free balance of the token for the specified account. You can adjust the API calls depending on the pallet used for the token module in your chain.

@hamidroohi92
Copy link
Collaborator

Please give me some tokens for testing

@hackfisher
Copy link
Contributor Author

Please give me some tokens for testing

sent

@kentaurse
Copy link

kentaurse commented Oct 17, 2024

hello @hackfisher
After reviewing the project, I’d like to propose a few enhancements that could further improve both the technical foundation and the user experience:

  1. Automated Token Balance Testing: Introducing automated tests to validate token balances across various account scenarios could ensure consistency and reliability during updates or code changes. This would provide peace of mind, especially as the system scales.

  2. Modular Error Handling for Token Retrieval: Improving error messaging for token retrieval failures by making them more specific and actionable could significantly reduce debugging time and enhance developer productivity.

  3. UI/UX Loading Indicators: Implementing a loading indicator while querying token balances would improve the user experience by clearly showing when a process is ongoing, preventing confusion during any potential retrieval delays.

I believe these enhancements would align well with the team’s goals of maintaining robustness and providing a smooth user experience. I'd be happy to help implement these solutions and explore any further optimizations the team has in mind!

@boundless-forest
Copy link
Contributor

Nice, please feel free to submit a PR. Thanks in advance.

@kentaurse
Copy link

@boundless-forest
Thanks for your response.
I will submit a PR.
What do you use as your primary communication for your company?
Discord or Telegram?
Also, can you invite me to your developer community?

@boundless-forest
Copy link
Contributor

Welcome to our community ❤️ Join Tg: https://t.me/+36RGh5TCd141ZWQx

I will submit a PR.

Hold on. We have a plan to implement significant changes to this repo recently, including design and both front-end and back-end aspects. So it's better to keep an eye on the status before working on it. Let's talk on Telegram; I believe we can find something to collaborate on together.

@kentaurse
Copy link

@boundless-forest
Thank you

@kentaurse
Copy link

@boundless-forest
I hope you’ve been well. I've recently transitioned to freelance work after completing my previous contract, and I’m very interested in your project. I’d love the opportunity to contribute and discuss the details further. If possible, could you invite me to your Discord server? It would be great to connect and talk more specifically there.

@boundless-forest
Copy link
Contributor

I'm glad to hear that! I remembered inviting you to our ecosystem developer group. Would you like to take another look? We have a collaboration repo at https://github.com/ringecosystem/collaboration; check out the README to see how we collaborate!

@kentaurse
Copy link

Thank you for your help.

@kentaurse
Copy link

Hi @boundless-forest
After a thorough review of the PreParaLink contract and deployment script, I identified several security vulnerabilities, functional limitations, and optimization opportunities.
So I tried to modify the code and push it, but I don't have permission. Could you invite me to the repository and give me permission?
image

@hackfisher
Copy link
Contributor Author

Hi @boundless-forest

After a thorough review of the PreParaLink contract and deployment script, I identified several security vulnerabilities, functional limitations, and optimization opportunities.

So I tried to modify the code and push it, but I don't have permission. Could you invite me to the repository and give me permission?

image

Please fork and use PR

@kentaurse
Copy link

@hackfisher
I got it.
Thank you.

@kentaurse
Copy link

@hackfisher
PR

Here’s an overview of the key improvements made:

  1. Enhanced Security for Contract Upgrades:
    The upgrade function previously relied solely on onlyOwner for access control, which posed a risk if ownership were transferred or compromised. I introduced a onlyUpgradeManager modifier to the _authorizeUpgrade function. This modifier requires multi-signature authorization for any contract upgrade, ensuring only authorized and verified entities can approve such critical changes. This update significantly reduces the risk of unauthorized upgrades and enhances the contract’s security.

  2. Generalized Token Transferability:
    The original implementation limited the transfer, transferFrom, and approve functions to owner-only access, which constrained the token’s use and was not aligned with standard ERC20 behavior. I removed the onlyOwner restriction, allowing any token holder to freely transfer and approve tokens. This modification broadens the token’s utility and ensures compliance with ERC20 standards, making it more functional and user-friendly.

  3. Increased Transparency with Initialization Event:
    To improve tracking of initial token distributions, I added an Initialized event in the initialize function. This event is emitted when tokens are minted to the initial owner, providing an on-chain record of this significant state change. By logging the initial distribution, this addition supports better transparency and auditing capabilities for the contract.

  4. Reentrancy Protection on Transfer Functions:
    Although the contract inherits from OpenZeppelin’s ERC20Upgradeable, which includes reentrancy protection, adding a nonReentrant modifier to the transfer and transferFrom functions provides an additional layer of security against potential reentrancy attacks. This enhancement fortifies these functions, especially in an upgradeable contract context, where reentrancy risks can sometimes be more challenging to track.

  5. Adaptability in Deployment Script:
    The Deploy script previously hardcoded the treasury address, which limited flexibility. To make deployment adaptable across environments, I replaced the hardcoded address with a constructor parameter, making it easier to specify the treasury address at runtime. Additionally, I added address validation checks for the deployed proxy and logic contracts to prevent potential deployment errors. This adjustment makes deployment more robust and environment-agnostic.

These updates collectively strengthen the contract’s security, compliance, and usability. They address potential vulnerabilities, optimize functionality, and make the code more resilient against misuse or unauthorized changes. The pull request is ready for review, and I look forward to your feedback on these improvements.

@hujw77
Copy link

hujw77 commented Nov 8, 2024

@hackfisher PR

Here’s an overview of the key improvements made:

  1. Enhanced Security for Contract Upgrades:
    The upgrade function previously relied solely on onlyOwner for access control, which posed a risk if ownership were transferred or compromised. I introduced a onlyUpgradeManager modifier to the _authorizeUpgrade function. This modifier requires multi-signature authorization for any contract upgrade, ensuring only authorized and verified entities can approve such critical changes. This update significantly reduces the risk of unauthorized upgrades and enhances the contract’s security.
  2. Generalized Token Transferability:
    The original implementation limited the transfer, transferFrom, and approve functions to owner-only access, which constrained the token’s use and was not aligned with standard ERC20 behavior. I removed the onlyOwner restriction, allowing any token holder to freely transfer and approve tokens. This modification broadens the token’s utility and ensures compliance with ERC20 standards, making it more functional and user-friendly.
  3. Increased Transparency with Initialization Event:
    To improve tracking of initial token distributions, I added an Initialized event in the initialize function. This event is emitted when tokens are minted to the initial owner, providing an on-chain record of this significant state change. By logging the initial distribution, this addition supports better transparency and auditing capabilities for the contract.
  4. Reentrancy Protection on Transfer Functions:
    Although the contract inherits from OpenZeppelin’s ERC20Upgradeable, which includes reentrancy protection, adding a nonReentrant modifier to the transfer and transferFrom functions provides an additional layer of security against potential reentrancy attacks. This enhancement fortifies these functions, especially in an upgradeable contract context, where reentrancy risks can sometimes be more challenging to track.
  5. Adaptability in Deployment Script:
    The Deploy script previously hardcoded the treasury address, which limited flexibility. To make deployment adaptable across environments, I replaced the hardcoded address with a constructor parameter, making it easier to specify the treasury address at runtime. Additionally, I added address validation checks for the deployed proxy and logic contracts to prevent potential deployment errors. This adjustment makes deployment more robust and environment-agnostic.

These updates collectively strengthen the contract’s security, compliance, and usability. They address potential vulnerabilities, optimize functionality, and make the code more resilient against misuse or unauthorized changes. The pull request is ready for review, and I look forward to your feedback on these improvements.

ringecosystem/PREPLK#2 (comment)

@kentaurse
Copy link

@hackfisher @boundless-forest @hujw77
Thank you for your kind words—they mean a lot, especially coming from such an accomplished team. Your project’s depth and technical rigor have truly impressed me, and I can tell there’s a strong emphasis on quality, security, and innovation here.

With years of experience in web and blockchain development, I’m passionate about contributing to teams that push boundaries, and I genuinely feel that working with your team would be an incredible opportunity for growth. The expertise and vision here are exactly the kind of environment where I know I’d thrive and continue developing my skills at a high level.

If there’s an opportunity for a full-time role, I would be thrilled to bring my experience and dedication to your projects. I’m eager to help drive the team’s vision forward, and I’d be deeply committed to contributing in any way possible. Thank you for considering me—I look forward to the possibility of joining such an exceptional team.

@kentaurse
Copy link

@boundless-forest
How do I get hired as a full-time developer?

@boundless-forest
Copy link
Contributor

Sorry for that. We currently don't have plans to hire another full-time developer. Stay tuned I‘ll let you know if there is a role later.

@kentaurse
Copy link

@boundless-forest
Thank you for your reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates a defect or error in the code that needs to be fixed. high priority Tasks that require immediate attention and should be addressed as soon as possible.
Projects
Development

No branches or pull requests

6 participants