[fastx types] Add gas coin type #97

sblackshear · 2021-12-29T00:40:38Z

This bridges one useful object type from the Move world to the Rust one. Transfer orders now act over objects whose type is "gas coin".

Introduce new GAS module in Move + use it to instantiate the generic Coin type. The resulting type for a gas token is Coin::Coin<GAS>,
Introduce Rust wrappers for the Move Coin, Coin<GAS>, and ID types
Use Coin<GAS> as the type of all test objects constructed by Object::with_id_owner_for_testing. The value of each test coin is 0, but we can change that if desired.
Restrict Transfer orders so they only work on gas coins.

patrickkuo · 2021-12-29T12:31:42Z

fastx_types/src/coin.rs

+    id::ID,
+};
+
+/// 0x7ABB80D444EB9F84F0CF64CC34CF8760


is this a special magic value from move?

This is the ObjectID of the Coin module that gets published at genesis.

Right now, the ID's for all the genesis modules get derived programmatically in the same way they do for ordinary objects (i.e., id = hash(tx_digest || N) for the Nth id created by the tx. But if we wanted to, we could hardcode special ID's for each genesis module (and there could be UX benefits for doing so).

Realized it is probably helpful for me to share how I knew to type this magic number (and how others in the future can figure it out if they need to add new Rust wrappers of Move types):

Write your wrapper type and use a dummy address (e.g., copy over COIN_ADDRESS)

Add a new conditional branch for your type along the lines of the code in 54--84 in genesis.rs that checks your dummy address

Run the tests, which will exercise your conditional branch and trip the assertion because your dummy address will not match the actual address of the type

Take the address printed by the test harness and add it to your type. Now, everything will work

This is a bit annoying, but it works for now. In the future, we can hopefully solve this via codegen of these Rust wrappers of Move types.

lxfind · 2021-12-30T19:45:03Z

Why are we restricting transfers to gas object only?

lxfind · 2021-12-30T19:29:41Z

fastx_programmability/adapter/src/genesis.rs

@@ -54,7 +51,37 @@ fn create_genesis_module_objects() -> Result<Genesis> {
            let self_id = m.self_id();
            // check that modules the runtime needs to know about have the expected names and addresses
            // if these assertions fail, it's likely because approrpiate constants need to be updated
-            if self_id.name() == TX_CONTEXT_MODULE_NAME {


Probably cleaner using match instead of if elses? Or if you want to make it even simpler, define a map from name to address, and fold all these if/else into one.

Not related to your change, but in line 44 (now 47), should it be assert_eq!(native.1.as_ident_str(), new_id.name()); instead? I don't expect module rewriter to change their names.

Probably cleaner using match instead of if elses? Or if you want to make it even simpler, define a map from name to address, and fold all these if/else into one.

I agree, but couldn't figure out how to match against a constant (not sure if Rust lets you do that?), but map idea is great--will make that fix.

Not related to your change, but in line 44 (now 47), should it be assert_eq!(native.1.as_ident_str(), new_id.name()); instead? I don't expect module rewriter to change their names.

Fair point. The rewriter can change the name if you ask it to (by providing a new name in the sub_map), but we don't use it that way here and shouldn't. Might as well make the assertion more precise to reflect that. Will make that change here if it's not too distracting.

Not related to your change, but in line 44 (now 47), should it be assert_eq!(native.1.as_ident_str(), new_id.name()); instead? I don't expect module rewriter to change their names.

My bad--I mistakenly thought I understood this, but I don't think I do. Which assertion in the original code is this referring to?

I think your previous understanding is correct. I was referring to this line:

native.1 = new_id.name().to_owned();

and suggest it to be assertion instead.

sblackshear · 2021-12-31T04:07:21Z

Why are we restricting transfers to gas object only?

Good question. User-defined objects can (and will want to) enforce restrictions on transfers, and we don't want to allow the native Transfer op to subvert those restrictions.

However, I don't think we'll want to restrict transfers to gas objects only--those are just the only type of objects we have right now. In the longer term, I think what probably makes sense is allowing native transfer for

Coin<T> for any T (and now that you mention it, we could implement that policy now)
Any of our standard libary types whose Move logic allows unconditional transfers (e.g., we will likely have mocks of ERC721 and ERC1155 that meet this condition)
[speculative] On the Move team, there has been some discussion adding custom attributes[1] to the bytecode language that are not interpreted by the VM or Move verifier, but can be given meaning by custom verifier passes. If this feature gets added, you could imagine supporting a transfer struct attribute for allowing unconditional transfers that allows the native transfer operation for your struct.

[1] See, e.g., diem/diem#10104, which is using the source attributes (which already exist) and mentions exposing them in the bytecode

This bridges one useful object type from the Move world to the Rust one. `Transfer` orders now act over objects whose type is "gas coin". - Introduce new `GAS` module in Move + use it to instantiate the generic `Coin` type. The resulting type for a gas token is `Coin::Coin<GAS>`, - Introduce Rust wrappers for the Move `Coin`, `Coin<GAS>`, and `ID` types - Use `Coin<GAS>` as the type of all test objects constructed by `Object::with_id_owner_for_testing`. The value of each test coin is 0, but we can change that if desired. - Restrict `Transfer` orders so they only work on gas coins.

sblackshear requested a review from oxade December 29, 2021 00:40

sblackshear force-pushed the gas_token branch 2 times, most recently from 99110fb to 1d30833 Compare December 29, 2021 06:19

sblackshear requested review from lxfind and patrickkuo December 29, 2021 06:29

patrickkuo reviewed Dec 29, 2021

View reviewed changes

lxfind reviewed Dec 30, 2021

View reviewed changes

lxfind approved these changes Dec 31, 2021

View reviewed changes

sblackshear force-pushed the gas_token branch from 1d30833 to 510a7d4 Compare December 31, 2021 04:32

sblackshear force-pushed the gas_token branch from 510a7d4 to 5efeaa9 Compare December 31, 2021 17:21

sblackshear merged commit 7f37727 into MystenLabs:main Dec 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fastx types] Add gas coin type #97

[fastx types] Add gas coin type #97

sblackshear commented Dec 29, 2021 •

edited

Loading

patrickkuo Dec 29, 2021

sblackshear Dec 29, 2021

sblackshear Dec 29, 2021

sblackshear Dec 30, 2021

lxfind commented Dec 30, 2021

lxfind Dec 30, 2021

lxfind Dec 30, 2021

sblackshear Dec 31, 2021 •

edited

Loading

sblackshear Dec 31, 2021

lxfind Dec 31, 2021

sblackshear commented Dec 31, 2021

[fastx types] Add gas coin type #97

[fastx types] Add gas coin type #97

Conversation

sblackshear commented Dec 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxfind commented Dec 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sblackshear Dec 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sblackshear commented Dec 31, 2021

sblackshear commented Dec 29, 2021 •

edited

Loading

sblackshear Dec 31, 2021 •

edited

Loading