merge syscall changes to master #533
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stebalien
force-pushed
the
feat/syscall-changes
branch
from
c9d7783 to
30bdb30
Compare
|
Actors: Conformance tests pass with patches. Now I'm going to try it on mainnet. |
|
This syncs mainnet after #562. |
Stebalien
force-pushed
the
feat/syscall-changes
branch
from
30bdb30 to
9823e37
Compare
|
This should be ready to merge. |
This makes it a bit easier to add support for additional hash functions later. We could just have many syscalls, but parameterizing over a multicodec makes it easier to implement/use multihash libraries in actors. fixes #270
Don't pass signatures as cbor-encoded objects with signature types. Instead, pass the type and the signature bytes separately. This change _also_ avoids copying the signature when verifying it. fixes #511
- We don't use it. - We're not charging for the copy/memory. - Tracing is better for this kind of thing.
We'll be adding more error numbers as we add additional syscalls. Adding new error numbers to the system shouldn't be a breaking change. NOTE: adding/changing an error number on a specific syscall is a different matter. But if we add a new syscall with a new error number, that's not breaking anything.
The _actors_ care about them, but we don't. This: 1. Reduces the coupling between actors & the FVM. 2. Generalizes these methods for user actors.
- Renames `resolve_builtin_actor_type` to `get_actor_builtin_type`.
There's no real resolution here.
- Make `resolve_address` fail with NotFound if the target actor doesn't
exist.
- Make `get_actor_code_cid` take an actor id, not an address.
- We don't charge for address resolution here.
- The address is almost always resolved anyways (i.e., the sender).
- Make CID return logic consistent:
- Fail with `ErrorNumber::BufferTooSmall` if it doesn't fit into the
output buffer.
- Always return the size of the CID as the return value.
- Never write partial CIDs only to fail later.
- Always check that we can write the return result before we execute the
syscall.
- More generally, be pedantic about checking for errors up-front. If a
syscall fails, it must fail _without_ side effects.
1. Fix/simplify error checks on put. 1. The overflow checks were wrong. 2. We didn't check for CBOR. 2. Refactor errors and implement conversions. This will make the next patch easier.
Previously, it would return the amount of data read. Unfortunately, this made it impossible to tell if we were at the end of the block. Now, we return the offset (negative or positive) from the end of the block, to the end of the user provided buffer.
This combines two changes that are somewhat interlinked. Really, they're just hard to factor out into two different patches without a bunch of work. Motivation: 1. Remove copying costs from send/return. 2. Avoid calling "syscall" kernel methods in the call manager. 3. I _really_ hated that `Kernel::block_get` method. 4. Remove an extra call on send to "stat" the returned object. ---- 1. In the kernel, the `send` function now takes/returns a block ID. This moves all the parameter-relates logic into the kernel itself. Otherwise, the syscall has to make three kernel calls, all of which charge gas. 2. In the call manager, instead of passing return values & parameters as raw bytes, we pass them as `Option<kernel::Block>` where `kernel::Block` is a reference counted IPLD block (with a codec). This, let's us pass them around without copying, and lets us be very clear about codecs, empty parameters, etc. 3. In the syscalls, return the block length/codec from send to avoid a second syscall _just_ to look those up.
Stebalien
force-pushed
the
feat/syscall-changes
branch
from
d724130 to
5cc08bd
Compare
Fixes the naming consistency issues identified in #521, but does not address the confusion around block_link.
raulk
commented
May 16, 2022
|
Don't merge yet. I need to fix some fallout from the threading changes. |
Unfortunately, these now include RC-ed blocks, which can't cross thread boundaries.
(found by @wadealexc) Also validate hash function & length, just in case.
Stebalien
approved these changes
May 16, 2022
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes:
Fixes #542.
Fixes #453.
Fixes #738.
Fixes #506 (items 1-3, but good enough to close that issue).