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

Enable Threshold (1-of-N) Support in HTS Precompile #1069

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mgarbs
Copy link
Collaborator

@mgarbs mgarbs commented Oct 31, 2024

Enable Threshold Keys Support in HTS updateTokenKeys Precompile

This update adds support for using threshold keyswithin the updateTokenKeys HTS precompile function, allowing developers to set token keys with multiple signatories directly in smart contracts. While complex keys are supported in the SDK, this enhancement allows on-chain token management with multi-signature and threshold-controlled permissions, expanding flexibility for applications requiring decentralized control over token keys.

Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for hedera-hips ready!

Name Link
🔨 Latest commit 2e4a81d
🔍 Latest deploy log https://app.netlify.com/sites/hedera-hips/deploys/67a4cdf82412fa00082e300b
😎 Deploy Preview https://deploy-preview-1069--hedera-hips.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

HIP/hip-1069.md Outdated Show resolved Hide resolved
@mgarbs mgarbs requested a review from Nana-EC January 23, 2025 16:20
@mgarbs mgarbs requested a review from ty-swirldslabs January 29, 2025 17:25
@mgarbs
Copy link
Collaborator Author

mgarbs commented Feb 3, 2025

Hello everyone,
To keep up the momentum, I kindly request everyone to take some time out of your schedules and revisit the unresolved discussions, your inputs are incredibly valuable. We're always one step closer to our goal when we collaborate and share knowledge.

Thank you all once again for your contributions thus far. Looking forward to our continuous collaboration on this.

Cheers!
@ty-swirldslabs
@stoyanov-st
@Nana-EC
@Neurone

@Neurone
Copy link
Contributor

Neurone commented Feb 4, 2025

Hi @mgarbs , it seems to me we solved all the pending issues. From my PoV we can proceed.

@lukelee-sl
Copy link
Member

lukelee-sl commented Feb 5, 2025

From a technical point of view, I do not think this can be accommodated without system contract versioning as you would be potentially breaking existing contracts because of the need to redefine the KeyValue struct. If this is indeed the case, then we need to decide if this feature should be included in the next HTS contract version.

@stoyanov-st
Copy link
Contributor

I totally agree with @lukelee-sl, this is a great candidate for the contract versioning initiative.
Also, does it make sense to include the solidity struct changes with the HIP?

@Neurone
Copy link
Contributor

Neurone commented Feb 6, 2025

I agree that this is a good fit for the new contract versioning initiative.

@lukelee-sl I don't think we need to make an explicit reference to this detail in the HIP, especially if the new process is going to be the new norm, but just in case, feel free to suggest some changes.

@stoyanov-st Yes, we need to make as many details explicit as possible, but they can be just an example and the real struct can be defined later. The guidelines for a HIP are to make explicit any changes that affect the operation of the network, including interfaces. In this case, if we maintain compatibility with old contracts, as we will now do with the versioning system, we can more easily skip all the details on the Solidity side, but it's good to have them as an initial idea and as a reference for developers reading the HIPs in the future. If there are major changes during development, we can always update the HIP with the correct interfaces, but if there are major changes, we will need to do a new HIP release.

@david-bakin-sl david-bakin-sl changed the title Enable KeyList Support in HTS Precompile Enable Threshold (1-of-N) Support in HTS Precompile Feb 7, 2025
@stoyanov-st
Copy link
Contributor

After carefully reviewing this HIP, the smart contracts team has agreed on several discussion topics that need to be addressed for the clarity and understanding of this feature. Those are:

  1. Solidity Structure Updates
  • A review of existing Solidity structures is necessary to ensure compatibility with the proposed changes.
  • Notably, due to recursion issues in KeyList we must determine whether to modify existing structures or introduce new ones to handle key management more efficiently.
  • If we follow the protobuf definitions for the structs modification there would be additional validation for the threshold = 1 that would require an appropriate response code for the unsuccessful scenarios.
  • Also, KeyHelper.sol needs adjustments.
  1. Other Threshold Limitations
  • While a threshold of 1 is the obvious limit, we need to address the more complex scenarios, such as a threshold within a threshold.
  • Defining these limitations early will help avoid ambiguity in implementation, misinterpretations and potential security concerns.
  1. Specification of Key Usage for Signing Transactions
  • A clear specification is needed regarding who signs transactions and which keys are used in different scenarios or a reference to an existing specification.
  1. System Contract Address
  • As this feature is going to be leveraging the System Contract Versioning, there needs to be a section stating the address that is going to be exposing it.

Considering the impact that this proposal is going to make, we need to be careful and also be able to foresee most of the implications that it is going to produce.

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

Successfully merging this pull request may close these issues.

5 participants