introduce an example ERC20-like token actor #290

raulk · 2022-01-21T16:35:02Z

This is an extremely rough-and-ugly implementation of an ERC20-like native token actor for the Filecoin network. It uses no SDK sugar (because none exists yet), neither for syscalls nor or error handling.

Supported features

The current code supports token symbols, names, maximum supplies, and simple transfers. It DOES NOT YET support authorizations for delegate spending.

On developer experience

As stated above, this is code is still ugly. The developer experience will improve dramatically as the FVM project advances towards Milestone 2.

The author deliberately didn't use this opportunity to improve the SDK. Instead, he chose to implement things low-level to create a baseline of where we are. As we introduce better DX, this actor will evolve and benefit.

NOTE: There is some pre-existing sugar in the actor/runtime module that could facilitate things slightly easier here, but this actor does not use it to avoid dragging in unnecessary dependencies (related to built-in actors). The upcoming sugar will eventually be provided by the Rust SDK, and not the built-in actors toolkit.

Boilerplate

The following responsiblities are taken care of here, but they are absolutely boilerplate, and should eventually be superseded by clever sugar in the form of proc macro attributes.

State loading.
State mutations.
Method dispatch.
Deserialization of parameters.
Serialization of return data.

Addressing

For simplicity, this actor uses ActorIDs in the balances and allowances HAMT (map) keys. This poses two problems:

A sender cannot send tokens to an inexistent account actor.
This actor is not reorg-safe (ActorIDs can change in the chain gets reorg-ed).

We could do sophisticated things here to solve for these problems, by associating balances to class-{1,2,3} addresses only, and rejecting calls with class-0 (ID addresses).

However, all of this will change when we introduce class-4 addresses (reorg stable universal addresses), so it's not worth the complexity now, as this actor only exists for illustration purposes.

⚠️ 🐉 🔥 ☠️ NOT MEANT FOR PRODUCTION USAGE

This actor should NEVER be deployed on the Filecoin network. It is purely meant for illustration and test purposes.

TODO

Integration hooks in the FVM to load custom code (test mode only).
Integration tests that deploy the actor into the FVM and play with it.

This allows custom actors to use a blockstore directly with, for example, AMT and HAMT data structures. The ActorBlockstore delegates to the SDK blockstore, and translates errors to exit codes, thus preserving the previous interface.

(Extracted and slightly reworded from the actor's module docs) This is an extremely rough-and-ugly implementation of an ERC20-like native token actor for the Filecoin network. It uses no SDK sugar (because none exists yet), neither for syscalls nor or error handling. The current code supports token symbols, names, maximum supplies, and simple transfers. It DOES NOT YET support authorizations for delegate spending. As stated above, this is code is still ugly. The developer experience will improve dramatically as the FVM project advances towards Milestone 2. The author deliberately didn't use this opportunity to improve the SDK. Instead, he chose to implement things low-level to create a baseline of where we are. As we introduce better DX, this actor will evolve and benefit. NOTE: There is _some_ pre-existing sugar in the actor/runtime module that could facilitate things slightly easier here, but this actor does not use it to avoid dragging in unnecessary dependencies (related to built-in actors). The upcoming sugar will eventually be provided by the Rust SDK, and not the built-in actors toolkit. The following responsiblities are taken care of here, but they are absolutely boilerplate, and should eventually be superseded by clever sugar in the form of proc macro attributes. - State loading. - State mutations. - Method dispatch. - Deserialization of parameters. - Serialization of return data. For simplicity, this actor uses ActorIDs in the balances and allowances HAMT (map) keys. This poses two problems: 1. A sender cannot send tokens to an inexistent account actor. 2. This actor is not reorg-safe (ActorIDs can change in the chain gets reorg-ed). We _could_ do sophisticated things here to solve for these problems, by associating balances to class-{1,2,3} addresses only, and rejecting calls with class-0 (ID addresses). However, all of this will change when we introduce [class-4 addresses](https://github.com/filecoin-project/fvm-specs/blob/main/04-evm-mapping.md#proposed-solution-universal-stable-addresses) (reorg stable universal addresses), so it's not worth the complexity now, as this actor only exists for illustration purposes. This actor should NEVER be deployed on the Filecoin network. It is purely meant for illustration and test purposes.

Guarded by the custom_actors Cargo feature.

anorth · 2022-01-24T01:08:29Z

examples/token/src/lib.rs

+            "failed to set new recipient balance in balances hamt: {:?}",
+            err
+        )
+    }


This will print tokens from thin air if the recipient is the sender. Writing the recipient balance overwrites the reduced sender balance.

An example of a safe sequence is https://github.com/filecoin-project/specs-actors/blob/master/support/vm/vm.go#L546

Well spotted. Fixed in 59cc833 (#290), along with other logic changes.

The goal of this exercise is not to achieve complete safety, but rather to illustrate the current state of deploying custom actors for local testing. But it's good to know that we have a safe sequence others can use as a reference.

This reverts commit a617abd.

…ctor

laudiacay · 2022-01-25T16:13:18Z

examples/token/src/lib.rs

+use ipld_hamt::Hamt;
+
+/// A macro to abort execution by signalling a non-zero exit code.
+macro_rules! abort {


probably should be in a library.

openzeppelin but make it FVM

Agree, these are just basic local macros because I'm not convinced this is what would ultimately go in an SDK in this particular shape. So I'm not inclined to "bless it" at this stage.

laudiacay · 2022-01-25T16:13:30Z

examples/token/src/lib.rs

+macro_rules! encode {
+    ($ex:expr) => {
+        match RawBytes::serialize($ex) {
+            Ok(ret) => ret,


also should be in a library

We probably want something nicer than this in a library. I.e., abstractions sends that auto-serialize/deserialize.

Same as above, and agree with @Stebalien. The code in this actor is deliberately dumb and low-level, a explained in the PR description, because the SDK needs careful thought.

laudiacay · 2022-01-25T16:16:22Z

examples/token/src/lib.rs

+    #[serde(with = "bigint_ser")]
+    pub max_supply: TokenAmount,
+    // map[ActorID] => TokenAmount
+    pub balances: Cid,


are we planning to just use tokenamounts here? no specification of decimals? I think it might be smart to make this a little bit more ERC20 compatible, especially if we want a defi ecosystem possibly getting built on FVM... the possibility of floating point issues in conversions down the road haunts me

yeah on further thought it's probably smart to have a decimals field added here too, unless I'm misunderstanding something deeply

+1. I've been looking into the translation of Eth token standards to FVM. @laudiacay would you like to work with me on defining an ERC20-like in a FIP?

could? this is already exactly that modulo decimals and approved

AFAIK decimals is actually an optional extension to ERC20. And yes, I agree it should go here because it makes the UX token nicer.

laudiacay · 2022-01-25T16:18:18Z

examples/token/src/lib.rs

+    // TODO should never use ActorID as allowees, since it's an unstable address
+    //  make sure to check for this
+    //  map[ActorID] => map[Address]TokenAmount
+    // pub allowances: Cid,


how are we planning to go from "we have like 4 kinds of addresses" to "we have one kind of address that we can do lookup tables over"? is there some other speedy solution for affiliating multiple cryptographic identities and related permission delegation that we're thinking about?

coming back to this after some comments further down: is there any design possibility of bonking everything except some curve's pubkeys (which function as the only UIDs in the system) completely out of userland? I need to look at how the syscall boundary is functioning right now to say anything intelligent about this, but it'd probably be really nice for developers if this was a thing we could do.

Currently everything unifies to an "id" address. I'd like to make all syscalls except "resolve_address" take an ID address, but, unfortunately, send also needs to be able to take a public-key address because sending to an account that doesn't exist magically creates that account.

what if a failed resolve creates an ID and returns, and you just resolve in userspace before the send?

ID addresses are not the solution because they are reorg-unstable. Because ID assignments can be reorged away, it is not safe to operate with a newly created ID address (e.g. an account actor, a miner, a payment channel) until finality (900 epochs). Otherwise, you could end up sending tokens to an address that gets reorg'ed away from you.´

The solution is to introduce class 4 addresses -- a universal, stable namespace for addresses: https://github.com/filecoin-project/fvm-specs/blob/main/04-evm-mapping.md#proposed-solution-universal-stable-addresses.

But we don't have those yet; that's why the token actor here uses the only other universal address option we have: the ID address, noting the caveat in various places in the code.

ID addresses are not the solution because they are reorg-unstable. Because ID assignments can be reorged away, it is not safe to operate with a newly created ID address (e.g. an account actor, a miner, a payment channel) until finality (900 epochs). Otherwise, you could end up sending tokens to an address that gets reorg'ed away from you.´

This isn't generally true. You don't want to use an ID address in a message, but state isn't re-org stable anyways. I.e.:

Use some form of stable address in a message.

Resolve to an ID address on-chain.

Use the ID address in on-chain datastructures.

laudiacay · 2022-01-25T16:19:02Z

examples/token/src/lib.rs

+
+    // Conduct method dispatch. Handle input parameters and return data.
+    let ret: Option<RawBytes> = match sdk::message::method_number() {
+        1 => Some(name(state)),


ought to have some kind of enum here, probably

otherwise/regardless, possibly ought to make this dispatch table clearer somehow... right now it takes me a second of staring at the code to determine what each of the method numbers is doing

My preferred sugar for dispatching would be attribute macros.

laudiacay · 2022-01-25T16:50:17Z

examples/token/src/lib.rs

+        Some(cid) => {
+            /// The multicodec value for raw data.
+            const IPLD_CODEC_RAW: u64 = 0x55;
+            // TODO this is embarrassingly wrong, as hardcoding the actor version doesn't allow evolution.


agreed yikes

this could be wrapped in some kind of shiny clean "is_an_account" function for developers, or we could not expose types of things that should never have ERC20s sent to them to developers.

or, we can just go totally hands-off, remove this check, and let them send erc20s to whatever they want to. that's how EVM does it, and developers are just expected to not be dummies about it and only send ERC20s to accounts that can handle them. transfer to the zero account or to contracts with no ability to ever withdraw or approve spends is not something you'd ever think to forbid in a basic ERC20 example. here, you aren't blocking sends BURN_FUNDS_ADDRESS, so there's something resembling a case for "why bother blocking sends to miner actors"?

laudiacay · 2022-01-25T16:50:52Z

examples/token/src/lib.rs

+
+    // Update balances.
+    sender_bal.0 -= &params.amount;
+    recipient_bal.0 += &params.amount;


do tokenamounts overflow? they're bigints, i guess not?

examples/token/src/lib.rs

laudiacay · 2022-01-25T17:02:35Z

examples/token/tests/token.rs

+}
+
+#[test]
+pub fn test_fail_transfer_to_non_account_actor() -> Result<()> {


these tests look fine to me for the current code... gonna need to add more to them when we add approvals, though

laudiacay · 2022-01-25T17:03:49Z

examples/token/src/lib.rs

+    // TODO should never use ActorID as allowees, since it's an unstable address
+    //  make sure to check for this
+    //  map[ActorID] => map[Address]TokenAmount
+    // pub allowances: Cid,


https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit

this frontrunning attack- eventually do we want to change the approval interface to their suggestion? alternatively, the erc20 token standard has some guidance for how userspace apps should be designed in order to prevent this.

I just had a similar discussion about a related flow in this FIP draft about authorizing a beneficiary address to receive some quota of miner rewards. The solution there was to record two numbers in state.

The single-field CAS method appears more state-space-efficient, whereas the two-fields solution has better usability properties in terms of reporting.

@laudiacay I think implementing the CAS method makes sense here when we implement approvals.

Stebalien · 2022-01-26T05:52:41Z

examples/token/src/lib.rs

+macro_rules! encode {
+    ($ex:expr) => {
+        match RawBytes::serialize($ex) {
+            Ok(ret) => ret,


We probably want something nicer than this in a library. I.e., abstractions sends that auto-serialize/deserialize.

Stebalien · 2022-01-26T05:55:01Z

examples/token/src/lib.rs

+    // map[ActorID] => TokenAmount
+    pub balances: Cid,
+    // TODO should never use ActorID as allowees, since it's an unstable address
+    //  make sure to check for this


We should definitely use ActorID in state, we just shouldn't use them in messages. That is:

User submits a message with some actor.

We resolve it on-chain.

We then store the ID in state.

If we re-org, we'll re-do this operation and store the new ID. The upside is that actor IDs are usually slightly cheaper to work with.

Stebalien · 2022-01-26T05:57:19Z

examples/token/src/lib.rs

+    // TODO should never use ActorID as allowees, since it's an unstable address
+    //  make sure to check for this
+    //  map[ActorID] => map[Address]TokenAmount
+    // pub allowances: Cid,


Currently everything unifies to an "id" address. I'd like to make all syscalls except "resolve_address" take an ID address, but, unfortunately, send also needs to be able to take a public-key address because sending to an account that doesn't exist magically creates that account.

Stebalien · 2022-01-26T06:15:13Z

examples/token/src/lib.rs

+        4 => {
+            let params: TransferParams = match params_cbor(params_id) {
+                Ok(params) => params,
+                Err(err) => abort!(ErrIllegalArgument, "failed to parse params: {:?}", err),


Yes. That and/or we can provide a custom panic handler (https://doc.rust-lang.org/nomicon/panic-handler.html) that exits with the "right" exit code.

Stebalien · 2022-01-26T06:15:30Z

examples/token/src/lib.rs

+/// The actor's WASM entrypoint. It takes the ID of the parameters block,
+/// and returns the ID of the return value block.
+#[no_mangle]
+pub fn invoke(params_id: u32) -> u32 {


We can have a lot of fun with the sugar:

#[derive(Serialize_tuple, Deserialize_tuple)] type Actor { name: String, value: u64, } #[fvm_actor] impl Actor { // Implicitly reads the state. #[fvm_export = 2] fn name(&self) -> &str { // takes nothing, returns a string &self.name } // Implicitly reads the state and writes it back #[fvm_export = 3] fn add(&mut self, value: u32) -> u32 { // takes a u32, returns a u32 self.value += value self.value } // Explicitly transform the state. We could use Deref... but that sounds dangerous. #[fvm_export = 4] fn sub(state: &mut Ctx<Self>, value: u32) -> u32 { // takes a u32, returns a u32 state.write(|st| { st.value -= value; }); state.read(|st| st.value) } }

We'll run into questions like:

Do we want some kind of FVM context object? For sending, syscalls, etc.

What about multiple parameters? Should we always encode parameters as a tuple? (I guess we already do).

this is EXTREMELY groovy....... i love the idea of pure actors that just do a state monad....... i love that soooooooo much..................

Yep, there's tons of exploration to do.

Stebalien · 2022-01-26T06:16:41Z

examples/token/src/lib.rs

+        abort!(ErrIllegalArgument, "cannot send to self");
+    }
+
+    // Ensure that the recipient is an account actor; otherwise they will never


non-account actors can definitely spend funds.

raulk added 2 commits January 21, 2022 16:28

move the syscall-backed Blockstore into the SDK.

6693253

This allows custom actors to use a blockstore directly with, for example, AMT and HAMT data structures. The ActorBlockstore delegates to the SDK blockstore, and translates errors to exit codes, thus preserving the previous interface.

raulk force-pushed the raulk/example-token-actor branch 4 times, most recently from 23dc505 to 3a90178 Compare January 21, 2022 20:07

machine: add support for loading custom actors from the blockstore.

7164cf8

Guarded by the custom_actors Cargo feature.

raulk force-pushed the raulk/example-token-actor branch from 3a90178 to 7164cf8 Compare January 21, 2022 20:24

anorth reviewed Jan 24, 2022

View reviewed changes

raulk added 7 commits January 25, 2022 00:21

remove unnecessary build files.

8ff05d7

make string state fields actual Strings.

49a06cb

token actor: prevent sending to self; prevent sending to non-accounts.

59cc833

token actor: add build script.

b98d277

token actor: add integration test.

6ca1f0b

remove example token actor from Makefile.

b7660ee

integrate into CI.

a617abd

raulk marked this pull request as ready for review January 25, 2022 00:30

raulk requested review from Stebalien and laudiacay January 25, 2022 00:31

galargh and others added 4 commits January 25, 2022 11:06

Revert "integrate into CI."

d7576e2

This reverts commit a617abd.

ci: allow custom package installation for cargo

bd48201

ci: extract fvm_token_actor tests

8b830b6

Merge pull request #296 from filecoin-project/galargh/example-token-a…

34de648

…ctor

laudiacay reviewed Jan 25, 2022

View reviewed changes

Stebalien reviewed Jan 26, 2022

View reviewed changes

Stebalien marked this pull request as draft May 5, 2022 14:47

raulk mentioned this pull request May 26, 2022

RFP Application: FVM - High level Rust SDK filecoin-project/devgrants#562

Closed

introduce an example ERC20-like token actor #290

Are you sure you want to change the base?

introduce an example ERC20-like token actor #290

Conversation

raulk commented Jan 21, 2022 • edited

Supported features

On developer experience

Boilerplate

Addressing

⚠️ 🐉 🔥 ☠️ NOT MEANT FOR PRODUCTION USAGE

TODO

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

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

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

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

Choose a reason for hiding this comment

raulk Jan 26, 2022 • edited

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulk commented Jan 21, 2022 •

edited

raulk Jan 26, 2022 •

edited