helix-onchain / filecoin Goto Github PK
View Code? Open in Web Editor NEWCollection of libraries to implement common patterns and standards on the Filecoin Virtual Machine
License: Other
Collection of libraries to implement common patterns and standards on the Filecoin Virtual Machine
License: Other
Given that this is shaping up to be a core shared library, it should live in the filecoin-project org:
Right now, there's an awkward dependency detour where:
Most changes to the FVM's API will require a change change in this repo before that change can be propagated to the builtin actors. This will be a non-issue once M2.2 (general programmability) ships as the FVM interfaces will have stabilized by then (or we'll have proper FVM "container" versioning, at the very least). But the current situation won't work for M2.1 and M2.2 because we're actively changing said interfaces.
The method hash macro uses blake2b-256, which is different than the FVM version (blake2b-512), changed in #89.
When fixing this, please create some "golden" tests for both the FVM and macro versions that include some hardcoded expected method number results that are computed via some separate code written in another programming lanuage.
This exposes an interesting chicken and egg problem.
In the proposal, I wrote:
An uninitialised pubkey address can receive tokens (because receiver hook will initialise it), but cannot be authorized as a spender
I was about to say you won't always have the actor ID so change to address, but I realise that we can't have the token state reflecting the new balances persisted if we don't know the receiving actor ID.
I think the required workaround will be that address resolution (in the actor code) needs to initialise actors if necessary. See resolve_to_id_addr
in the builtin-actors runtime/src/builtin/shared.rs
. And then this API is ok. (Defer this for a follow-up)
Originally posted by @anorth in #28 (comment)
We would like to write example actors that consume the fil_token
package and test the behaviour at the actor level
Currently actors within the ref-fvm repo are tested via the internal fvm_integration_tests
package. However, attempting to use that here causes compilation errors.
Currently it only has a single integration test that mints tokens, this should be extended (or new tests added) to involve allowances, transfers and burn so all the major operations are covered. Need to be careful because those tests are quite heavy and could lead to even longer CI runs.
The existing impl From<TokenError> for ExitCode
consumes the error value. This makes it slightly annoying difficult to both get an exit code and invoke to_string()
or do any other inspection of the error value.
The built-in actors use two statements to first inspect the message, then get the exit code at the last moment:
fn actor_result(self) -> Result<T, ActorError> {
self.map_err(|e| {
let msg = e.to_string();
ActorError::unchecked(ExitCode::from(e), msg)
})
}
It would be nice to find a way to get a code from a reference.
See the built-in actors and filecoin-project/builtin-actors#513 (comment) as inspiration.
Add a method to TokenState that checks all invariants. Call this at the end of every unit test, and expose it for use externally.
Some token library tests were ignored in #55 when the token receiver hook call was extracted. Move or rework the tests so that none are ignored.
I don't know if it's a good idea for the library code to be doing this instead of handling state loading on the user implementation side (like we're doing for fungible tokens). It'll be ok for now since it's going into a dev branch but I think it'll cause problems in actor code that carries additional state of its own beyond the NFTState
It does open up one easy optimisation though: we can pass through the original MintReturn
data in the intermediate struct and simply return that if there's no change once the hook call completes. I've been playing with the idea here (sending original data) and here (trying to centralise the 'has state changed' check) for the fungible token
Originally posted by @abright in #138 (comment)
A testing function that examines the HAMTs, AMTs within the NFT state and checks that all keys, balances, etc. are valid and consistent
This should be called at the end (and any point during the execution) of all state unit tests
Not much comments on Fungible Token Standards, do have a lot to say on Non Fungible Token Standards. But I do notice many new chains are using u128/u64 over BigInt, simply because they are primitives and more than sufficient to cover all use cases.
Originally posted by @Tom-OriginStorage in filecoin-project/FIPs#407 (comment)
For reverting state after hook call, benchmark the following strategies
Discussions of dropping anyhow in some of our dependencies may warrant re-evaluating if we want to opt our consumers in to this
filecoin-project/builtin-actors#214
filecoin-project/ref-fvm#463
Because rust doesn't let us derive builtin traits on external types this would make development a bit nicer. There's a need for message params (see here) but probably helpful for all exported types.
A simple example actor showing the bare minimum to get a compliant token running. It should have a public mint function that anyone can use to increase their own balance.
While not a standard actor method, users may want to directly specify an allowance.
Along these lines, think about any other basic CRUD methods the library shoudl expose for dev convenience (while maintaining its internal invariants).
The current address initialization/resolution logic has a few warts:
resolve_id
doesn't guarantee that the ID address is actually assigned. This isn't a huge issue, but can be very confusing.initialize_account
is no longer accurate. It will need to be something like initialize_address
.initialize_address
/initialize_account
. They should instead call resolve_or_create
/resolve_or_init
.IMO, we should have two methods:
resolve_existing
that asserts that the target ID actually exists.resolve_or_init
that tries to create the target ID.We can also just rename initialize_account
to initialize_address
...
All standardised method numbers/invocations should be handled by a single line of library code.
Unhandled method numbers are delegated to the actor author so that they can extend functionality (e.g. for minting)
To investigate the possibility of having a single code bundle on-chain that can be constructed with different params.
Simplest would be to have an initial account which is permitted to mint that can then be permanently revoked. Or simply a fixed supply to begin with distributed to specified account(s).
Currently when balances and allowances are updated, the entire map is read from the blockstore and then written back.
It may be more efficient to shard the balance map by ActorID so that the number of bytes written and read to/from the blockstore is lower. If sharding factor should be configurable if indeed it is overall more gas efficient.
If any map is changed, it will still be necessary to recompute the root Cid.
Provide high level overview of the various crates/usage scenarios
Splitting these up gives the user an opportunity to save (and commit) their state before calling the receiver hook, to guard against re-entrance attacks. The initial split is simple enough - make the mint/transfer functions return suitable parameters to forward to the receiver hook - but ideally we'd have it return some callable type that required the user to call the hook or abort the transaction entirely.
First part has already been implemented (#76), still need to add a callable type to enforce the receiver hook call and a number of tests will need to be adapted or rewritten for the new design
Looks like a CI run ends up building everything two or three times, which takes ages on the GitHub action runners. We should look into improving this when time allows. Might be time to split into multiple workspaces (eg: library vs actor code) or if there's enough actor code, maybe a separate repo?
Could be some easy wins running some build stages in parallel too?
Implement the change described in filecoin-project/FIPs#434
Follows on from #103 (comment)
The new model requires querying balance and allowance data separately after every mint or transfer is completed, in order to deal with possible actions taken inside the receiver hook. If state didn't change inside the receiver hook call, we could safely use the values we get from within the mint/transfer implementations.
It should be easy to pass through those values in the intermediate data so they are ready for use, the tricky part here is knowing when the state is 'dirty' and we need to query fresh values. We could save the root CID in there as part of making the hook call and compare it afterwards (in mint_return
) to decide but maybe there's a better way to do it?
You'll need a new release of the FVM SDK to get the blake-512.
@anorth has suggested
One consideration for improving the API is efficiency of state writes for bulk operations. The current API will force some unnecessary state.save() calls for intermediate states. E.g. intialising and then minting to a bunch of accounts up front, or making multiple transfers (from some non-standard token method). This API is pretty good for the basic cases and we want to keep them easy. We should think about how/whether to change this API to aid batching, or add an alternative style for less common but more complex calls. Maybe a closure that executes over state object, or a builder-style API that can abstract over it while batching.
In the built-in actors, callers currently do something like
token
.mint(operator, to, ¶ms.amount, RawBytes::default())
.exit_code(ExitCode::USR_UNSPECIFIED) // FIXME translate error to exit code
(where exit_code
is method on Result<T, std::error::Error>
that adapts to an ActorError
(built-in actors type that carries exit code and results in abort).
Without doing anything too specific to the built-in actors runtime, add a standard mapping of errors to exit codes, so that all token implementers will use consistent code by default. A free function doing the mapping could be sufficient.
Follow-up from issue #88 and PR #93 (comment)
The receiver hook itself isn't limited to FRC46 tokens anymore and should find use with NFTs and other assets. It's worth pulling that out into a separate crate so it's easier to share around and use without pulling in all the token code.
This issue comes from a review by @Kubuxu.
Token::set_balance
is the only place where the Token
's abstraction leaks the need for total_supply
accounting, and no utilities are provided to resolve that.
Suggestion: either turn set_balance
private or make it do total_supply
accounting or provide utilities to fix total_supply
.
This was added in #98 alongside set_allowance
, which is much easier to use properly.
See: #133 (comment)
Add a transaction like capability to the state level of the NFT library so that multiple state updates can be batched and the blockstore flushes can be delayed.
This creates a FIP-??? compliant fungible token that wraps the value of of
native FIL.
Calling the mint
(non-standard) method on this actor will increase the
caller's WFIL balance by the amount of FIL sent in the message. The WFIL can be
transferred between any FIP-??? compliant wallets (which have token_received
hook) by either direct transfer or delegated transfer.
https://etherscan.io/tx/0x96a7155b44b77c173e7c534ae1ceca536ba2ce534012ff844cf8c1737bc54921
Call transfer(TransferParams)
to directly transfer tokens from the caller's
wallet to a receiving address.
increase_allowance(AllowanceParams)
to increase the "spenders"transfer_from(TransferFromParams)
toTransferring WFIL to the address of this actor itself will result in the WFIL
being burnt and the corresponding FIL being credited back. This prevents the
case where tokens are unintentionally locked in to contracts that are unable to
receive them. This flow requires the actor to implement its own token_received
hook.
However, also compliant with the token standard would be for this actor to omit
the token_received
hook. In this case, transfers to the contract itself would
simply be rejected, which also prevents unintentional loss of tokens.
The fungible token state uses ActorIDs directly as keys in the HAMTs in its state. This is not spec compliant because ActorIDs serialize as cbor ints. This is something that the HAMT library should have caught with type checking but unfortunately this is not yet the case (filecoin-project/ref-fvm#900).
For now the right way to handle this is to wrap integer serializing keys in a byte serializing type here's an example in the power actor.
Currently, we have supposed a bitwidth of 5 is ideal for the HAMTs.
This constant should be determined empirically by simulating actors under use with large balances and transfer volume.
It should probably also be overridable by consumers if they wish to customise.
The balance granularity is not stored in state, but supplied from outside. Check that all balances are whole multiples of the granularity.
I added these to avoid build errors with unresolved extern
s in non-WASM builds because the SDK syscalls pull them in under the hood and they won't be considered dead code if we're using anything touching the hashing or method call code. A better long term solution might be to integrate with the new fvm_integration_tests
package instead, or find some other means of dealing with them at the SDK level.
Callers of the state invariant check will often want to check additional app-specific invariants, including reconciliation with the state of other actors.
Return a summary of the state from check_invariants. This doesn't need to be CBOR-serializable.
See the FRC. Add the method to the library, and appropriate types/aliases, and to the example actor.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.