Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: read/write context/permission #270

Merged
merged 18 commits into from
Mar 27, 2023

Conversation

da1suk8
Copy link
Member

@da1suk8 da1suk8 commented Mar 6, 2023

Description

This PR is related to #263.

#[callable_points] macro (NOT #[callable_point]) was created to expose read-write permissions to WASM using the _get_callable_points_properties().

This macro is used as follows.

#[callable_points]
mod callable_points {
    use super::*; 

    #[callable_point]
    fn foo(_deps: DepsMut, _env: Env, ...) {} // exposed to WASM
    
    #[callable_point]
    fn bar(_deps: Deps, _env: Env, ...) {} // exposed to WASM

    fn foobar() {} // NOT exposed to WASM
}

#[callable_point] is used as a mark rather than an attribute macro.
Only those with #[callable_point] above the function will be exposed to WASM.
It is also used for functions that manage read-write permissions for each function.

In the above example, the _get_callable_points_properties() can be used to retrieve the following map.
(true is read-only, false is read-write)

{
    "foo": CalleeProperty { is_read_only: false }, 
    "bar": CalleeProperty { is_read_only: true }, 
}

I also added a contract called intermediate-number, which has the roles of callee and caller.
Only sub is intentionally made a function with read-write permission.

The code expanded in macro can be viewed below.

$pwd
./contracts/number
$cargo expand --target wasm32-unknown-unknown

First, it may be necessary to add cargo expand and the target of wasm32-unknown-unknown.

Closes #263

Types of changes

  • Bug fix (changes which fixes an issue)
  • New feature (changes which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • ETC (build, ci, docs, perf, refactor, style, test)

Checklist

Because created a function to export the read-write permission of each function
of callable_point outside
Added to check if read-write and read-only permissions
are properly inherited
As for sub, intentionally use handle-sub as a function
with read-wirte permission
@da1suk8 da1suk8 self-assigned this Mar 6, 2023
@da1suk8 da1suk8 marked this pull request as ready for review March 6, 2023 04:59
packages/derive/src/callable_point.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_point.rs Outdated Show resolved Hide resolved
packages/derive/src/lib.rs Show resolved Hide resolved
packages/derive/src/lib.rs Outdated Show resolved Hide resolved
because to make it easy to understand that thsi contract is
between call-number and number
divide into module
modified for better understanding
@da1suk8 da1suk8 requested a review from loloicci March 8, 2023 07:18
Copy link
Contributor

@loloicci loloicci left a comment

Choose a reason for hiding this comment

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

Write what the callable_points macro does in the docs.
"Function attributed with this macro must take deps..." is not suitable for this macro.

5d38732#diff-484ed4ec90b9bea6e333d2819a67f7b7421528d49e411d19fb93f25105e070abR96

packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/callable_points.rs Outdated Show resolved Hide resolved
packages/derive/src/lib.rs Outdated Show resolved Hide resolved
@loloicci
Copy link
Contributor

loloicci commented Mar 9, 2023

In this code, serialization of the map (name -> permission) is done in generated code. But, we can serialize it in macro (compile time) and use serialized binary in generated code.

rename __callable_point to callable_points
fix typo
add document
Co-authored-by: TAKASE Ryo <loloicci@loloicci.dev>
@da1suk8 da1suk8 requested a review from loloicci March 13, 2023 01:39
change to use serialized binaries
contracts/intermediate-number/.gitignore Outdated Show resolved Hide resolved
@Kynea0b
Copy link
Contributor

Kynea0b commented Mar 15, 2023

There is a typo in the command in Description. Please add n at the end.
wrong: $cargo expand --target wasm32-unknown-unknow
right:$cargo expand --target wasm32-unknown-unknown
@da1suk8

Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

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

LGTM

@da1suk8 da1suk8 added the dynamic_link relate the dynamic link call feature label Mar 20, 2023
@da1suk8
Copy link
Member Author

da1suk8 commented Mar 24, 2023

Finschia/wasmvm#93 (comment)

The read-write permission was confusing, so the structure has been modified.
The structure is as follows. (Ex: contract of number)

{
    "sub": CalleeInfo { is_read_only: false }, 
    "add": CalleeInfo { is_read_only: false }, 
    "number": CalleeInfo { is_read_only: true }, 
    "mul": CalleeInfo { is_read_only: false }
}

@da1suk8 da1suk8 requested review from dudong2 and Kynea0b March 24, 2023 02:49
Copy link
Contributor

@loloicci loloicci left a comment

Choose a reason for hiding this comment

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

Sorry for canceling approval, but recheck these comments.

#270 (comment)
#270 (comment)
I missed c337ae7, sorry.
Never mind.

@da1suk8 da1suk8 requested a review from loloicci March 24, 2023 05:09
Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

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

LGTM

@da1suk8 da1suk8 merged commit fc54ae1 into Finschia:dynamic_link Mar 27, 2023
@da1suk8 da1suk8 deleted the feat/read-write-context branch March 27, 2023 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dynamic_link relate the dynamic link call feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants