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

Fix: Bug related shielded action for transfer #734

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pnam1609
Copy link

@pnam1609 pnam1609 commented Apr 10, 2024

Link issue:

Bug 1: Wrong chain id passing for shielded IBC transfer ( only IBC shielded )

  • Currently in IBC transfer function get chain ID from account like image below
    image
  • The chain id in the account is always taken from the .env file. It will not be updated according to the chain ID extension. That's why the chain ID is wrong
  • Solution: Chain id should be get from redux: const chain = useAppSelector((state) => state.chain.config). This is the correct chain ID same as chain id of extension

Bug 2: Private key is not passing into shielded action ( both IBC shielded and internal shielded transfer )

  • At present, the private key is not being passed for shielded actions. The private key must be passed along with the transaction to serve as the payer for fees and to process the transaction. Failure to include the private key results in this bug.
  • Solution: We propose adding a parameter called transparentAddress specifically for shielded transfers. This transparentAddress parameter would facilitate the generation of the private key required for signing and paying for transactions. By including this parameter, ensuring that the necessary private key is provided, thus enabling seamless signing and payment for transactions.

Bug 3: Public key is missing in txArgs when execute shielded transfer ( both ibc shielded and internal shielded transfer )

  • Same as bug 2, the public key should be passed as an argument in the build_transfer function. This argument is crucial for determining the payer of the transaction fees.
  • Solution: Because the account don't have public key, so will mapping in redux to add public key into shielded account

Bug 4: disposable_signing_key should be false when using shielded transfer ( only internal shielded transfer )

  • The current flag disposableSigningKey is set based on isShieldedSource. From my understanding, this flag indicates whether the signing key is disposable, and the validator is responsible for paying the transaction fee. Therefore, it's advisable to enable this flag only for voting proposal transactions.
  • Solution: Remove this flag in transfer transaction
    image

@emccorson emccorson added the needs discussion in issue Issue needs to be discussed before PR can be reviewed label Apr 11, 2024
@pnam1609
Copy link
Author

I tried latest code and i can't query shielded balance. So my PR used a old commit to base it on. That reason why my code have a lot conflict file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion in issue Issue needs to be discussed before PR can be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants