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

Create hip-362-token-freeze-amount.md #395

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

Conversation

Topachi
Copy link

@Topachi Topachi commented Mar 9, 2022

Description:

Related issue(s):

Fixes #

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@netlify
Copy link

netlify bot commented Mar 9, 2022

Deploy Preview for hedera-hips ready!

Name Link
🔨 Latest commit 5b43f81
🔍 Latest deploy log https://app.netlify.com/sites/hedera-hips/deploys/62a4b8106d965c00090e8569
😎 Deploy Preview https://deploy-preview-395--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 settings.

Update the Rationale and User story as per discussion with the original author.
@mohsin-hedera
Copy link

@ToMaciANura updated the story and rationale per our Discord exchange.

@mgarbs
Copy link
Collaborator

mgarbs commented Apr 19, 2022

Could you check that the commits are signed off (failing DCO) DCO sign offs

@@ -0,0 +1,44 @@
**hip: TBD </br>
Copy link
Collaborator

@mgarbs mgarbs Apr 19, 2022

Choose a reason for hiding this comment

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

Suggested change
**hip: TBD </br>
---
hip: 395

Comment on lines +2 to +10
title: Integrate '.setTokenAmount()' custom function for 'TokenFreezeTransaction()' </br>
author: Tomachi Anura [shibartoken@protonmail.com](mailto:shibartoken@protonmail.com)</br>
type: Standards Track</br>
category: Service</br>
needs-council-approval: Yes</br>
status: Draft</br>
last-call-date-time:</br>
created: 22.02.13</br>
discussions-to:**</br>
Copy link
Collaborator

@mgarbs mgarbs Apr 19, 2022

Choose a reason for hiding this comment

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

Suggested change
title: Integrate '.setTokenAmount()' custom function for 'TokenFreezeTransaction()' </br>
author: Tomachi Anura [shibartoken@protonmail.com](mailto:shibartoken@protonmail.com)</br>
type: Standards Track</br>
category: Service</br>
needs-council-approval: Yes</br>
status: Draft</br>
last-call-date-time:</br>
created: 22.02.13</br>
discussions-to:**</br>
title: Integrate `.setTokenAmount()` custom function for `TokenFreezeTransaction()`
author: Tomachi Anura [shibartoken@protonmail.com](mailto:shibartoken@protonmail.com)
type: Standards Track
category: Service
needs-council-approval: Yes
status: Draft
last-call-date-time:
created: 22.02.13
discussions-to:**
---

Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a discussions for this and add the link to it in line 10

Copy link
Collaborator

@mgarbs mgarbs left a comment

Choose a reason for hiding this comment

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

Please see comments below

mohsin-hedera and others added 3 commits June 11, 2022 11:41
Update the Rationale and User story as per discussion with the original author.

Signed-off-by: tomacianura <shibartoken@gmail.com>
Signed-off-by: tomacianura <shibartoken@gmail.com>
@Topachi Topachi marked this pull request as ready for review June 11, 2022 15:47
Copy link
Collaborator

@mgarbs mgarbs left a comment

Choose a reason for hiding this comment

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

It looks like you have elaborated some things here and it looks good, but the formatting is still off and the header is missing things like a discussion link etc. We also dont need these </br> tags. Refer to hip-1 to help with how to format a hip properly

Lastly, we are still failing the DCO check. Once we resolve this stuff I'll give my approval. Thanks for your attention here.

@tomachianura
Copy link

It looks like you have elaborated some things here and it looks good, but the formatting is still off and the header is missing things like a discussion link etc. We also dont need these </br> tags. Refer to hip-1 to help with how to format a hip properly

Lastly, we are still failing the DCO check. Once we resolve this stuff I'll give me approval. Thanks for your attention here.

not sure what you guys are asking for, i can see all the commits have been properly signed:
Topachi@b9ac2d8

can you please be more precise with your request?

CleanShot 2022-06-16 at 12 05 35@2x

@mgarbs
Copy link
Collaborator

mgarbs commented Jun 16, 2022

can you please be more precise with your request?

Sure. If you check out this link you'll find there are 3 commits that are not properly signed off DCO sign-off failures

Also, the formatting is not right. If you go to this link and see

and compare it to hip-1 (our template hip) you will see the formatting is not in alignment with this template hip.

@mgarbs
Copy link
Collaborator

mgarbs commented Jun 28, 2022

Reviewer update:

Considering the cost of storage overheads wrt to our current architecture, freezing a set amount of tokens per account is not a priority at this time as it can be achieved using smart contracts. Thanks

@tomachianura
Copy link

Sorry, but I just don’t get it.
If Hedera as a network priorities smart contract porting more than native layer, we are very much screwed 😊
We need this feature in order to fully implement in an elegant way what a lock can be in Hedera's way.
Inviting us to use smart contract is not the way, neither is to use the smart contract feature as an excuse for ignoring nor bypassing this HIP at all.
In my humble opinion, every single feature you implement on a contract MUST come on a native layer as very first.
If this approach get lost, we will become just a shadow mirror of ETH after all

@HGraphPunks
Copy link

Implementing this functionality for our platform, to my understanding I'd have to build it with a SC and would then need to make sure that code is correct / solidity, and would not be able to achieve this through native Hedera.

If we break away from building something better and start requiring Hedera devs to learn solidity / learn ETH languages to perform a functionality, I don't see how that pushes the Hedera Network forward or helps developers that are committed to the Hedera Network.

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