Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

feat: add new UDT model which uses only one cell per wallet #10

Merged
merged 2 commits into from Dec 14, 2018

Conversation

xxuejie
Copy link
Collaborator

@xxuejie xxuejie commented Dec 14, 2018

Previously, our UDT model works on a set of cells in one account(like bitcoin model), this PR adds a new mode where we can use only one cell to represent a single account, which is like ETH model.

One novelty to this model is that it demonstrates how one can temporarily transfer his/her cell to a different user for valid manipulation.

token_info.name,
token_info.pubkey
Ckb::Utils.bin_to_hex(pubkey_bin)
Copy link
Member

Choose a reason for hiding this comment

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

Better refactor to use bin instead of hex in future, more space efficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, we can use a future PR for this.

lib/ckb/udt_wallet.rb Outdated Show resolved Hide resolved
]
}
}
]
Copy link
Member

Choose a reason for hiding this comment

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

I feel it would be better to check the sum of two outputs equals the original cell[:amount], to prevent integer overflow on line #355 in case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I totally agree we should prevent integer overflow, but I believe we should do the check at here in the validator. The commented code here is the generator part which should be relatively safe.

Thoughts?

# * The matched output has the same amount of capacity but more tokens
# than the input
# This would allow a sender to send tokens to a receiver in one step
# without needing work from the receiver side.
Copy link
Member

Choose a reason for hiding this comment

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

🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻 🍻
:trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface: :trollface:

Choose a reason for hiding this comment

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

Niubility... 👍 👍 💯 💯

@janx janx merged commit 9212638 into master Dec 14, 2018
@janx janx deleted the cell-wallet branch December 14, 2018 06:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants