-
Notifications
You must be signed in to change notification settings - Fork 0
PoC: modularize executor and simplify Wasm I/O with permissionless and permissioned scenarios #1
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
base: init
Are you sure you want to change the base?
Conversation
Signed-off-by: Shunkichi Sato <[email protected]>
Signed-off-by: Shunkichi Sato <[email protected]>
| }); | ||
|
|
||
| #[test] | ||
| fn instruction_flows() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reviewers,
First of all, please run cargo test -- --show-output at commit 85749d7 as described in the README, and review the output.
|
Can't comment yet about the fuzzy/flex keys and the new model of permissions in general, but the use of WASI and WIT is very intriguing. Particularly, WIT is a great candidate for defining the data model agnostically to Rust or Iroha (hyperledger-iroha/iroha#2021). |
WebAssembly Component ModelThis looks great. Pros:
Cons:
Batching IO
PermissionsIt is hard for me to see the trade-offs of the new and old approaches. I haven't been exploring the permission system closely. As a note, read keys (fuzzy and flex) could be combined into a notion of dynamic (or read) key with variants:
Tree-like structureThis is a drastic breaking change, as you say. It would require rewriting the majority of integration tests, the SDKs, huge chunks of the docs, the Explorer, and probably something else. I would call it a shift to Iroha 3. Whether this change is worth the effort - I don't know. The PR and the PoC don't show the entire impact of it. And I don't really understand how to work with it, TBH (need to dive deeper). SummaryI would be hesitant to proceed to such drastic changes. It requires huge effort across many areas, and I am not sure we are capable of doing so. Additionally, I am not sure the effort is worth it. What is the finite problem we are trying to solve? Is there a possibly more pragmatic way to achieve it with less cost? |
|
Thank you for the detailed review with proactive suggestions. I’ll comment on the most concerning breaking API changes and propose alternative approaches that might be more acceptable. Breaking API changes for events and permissions, no changes to queries and instructionsIn fact, this plan does not introduce any new API change proposals. Although I suggested that migrating to Wasm components would alter the current API, the changes to events and permissions were already explicitly stated when the RC2 milestone was established to enable fine-grained access control and batch processing. As for queries and instructions, I’m not necessarily arguing that their roles in the UI should be replaced by ReadSet and WriteSet. Adopting the bindings generated from WIT almost as-is in the UI would result in breaking API changes, but we can avoid that by providing conversions between those bindings and the existing structures. This involves modifying the conversions defined in Reconfirming the motivationKnown issues to be resolved: Bug fix (#5171)Currently, Performance improvement (#4756)The union of permissions will be passed to the authorizer only once (AllowSet). Missing feature blocking production adoption: permissions for querying transactions (#5338)By making the write range of transactions comparable with the reader’s readable range, this is addressed. Scope and commitments of this plan
With these conditions in place, may I proceed with the work? |
|
Thank you for clarification. I am still skeptical about the point of "reading everything before execution". As I said:
I do, however, find it useful to be able to batch queries together (related: hyperledger-iroha/iroha#5044). Would it be possible to not enforce smartcontracts to define unconditional inputs, but provide a way to read in batches dynamically instead? What are the trade-offs here? |
| unimplemented!("boilerplate"); | ||
| } | ||
|
|
||
| fn write_request(view: ViewSet, args: String) -> WriteSet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a thread for discussing the separation of the read side and the write side: #1 (comment)
We may be able to support adding as many
fn more_read_request(view: ViewSet, args: String) -> ReadSetas the user needs before moving on to write_request.
Even then, there’s value in batching wherever possible, and in most simple cases a single read will be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may be able to support adding as many
This sounds like adding an arbitrary amount of static methods that users may or may not use. I think this isn't a flexible design choice and a dynamic approach is preferable. It is also still hard to think about smart contract design in the sense of "multi-stage reading".
there’s value in batching wherever possible
Yes, but that's up to the implementer of a smart contract. I think we shall provide an instrument to make batched reads, but not enforce it, and not limit all reads to a fixed point of smartcontract execution. In other words, leave it the same as it is now, but add a mechanism to make batched queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it’s not common to design a smart contract that limits read access to a finite number before writing. The previous design—where reads and writes could be interleaved in any order and as many times as needed—is more typical, so I may need to revise my plan.
The original goal of this design was to minimize the performance overhead of permission checks on every access in Iroha (#4756). Currently, permission tokens are re-aggregated on each check, but in the PoC the union of permissions obtained on the first read access is cached as part of the instruction’s transition state and reused on the next write access.
This caching effect remains effective even as the number of accesses grows and may be sufficiently powerful. However, it would be even more ideal to cache the account permissions on the authorizer side rather than the host side, since that would suppress the problematic serialize/deserialize operations on the FFI boundary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, it would be even more ideal to cache the account permissions on the authorizer side rather than the host side
Do you have an idea on how to cache permissions on the authorizer side?
As I understand, that would require a "stateful authorizer (executor, validator) session" during the time of a single smartcontract execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, as long as we adhere to the component model, memory sharing isn’t allowed and the authorizer can’t cache, so de/serialization will be inevitable per request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design Revision Proposal
For write access, since allowing arbitrary invocations would add complexity without sufficient benefit, we plan to keep it limited to a single invocation. In more complex use cases, it will still be possible to include a subsequent Wasm instruction call within that single write access.
For read access, the outline of the planned changes is as follows:
Instead of having the guest define this read-request:
export read-request: func(args: string) -> read-set;the host will provide a generic read function:
import read: func(request: read-set) -> result<view-set, unauthorized>;The guest can use this read function any number of times to implement the write-request function:
export write-request: func(args: string) -> write-set;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the host could also provide a generic function for writes without adding extra complexity:
import write: func(request: write-set) -> result<event-set, unauthorized>;I’ll try out a few PoC implementations.
Context
Transcript from Jun 05 standup:
feat: demonstrate permissionless scenario
This commit corresponds to the point at which authorization functionality—introducing new concepts for events and permissions—remains only a stub. Please begin your review with this relatively simple phase, because jumping straight to the latest commit might be overwhelming. This commit already captures the vision for Wasm I/O (#5358) very well.
Review notes
The concept of fuzzy keys may be confusing. They act as containers for capturing their counterparts (see the
fuzzy_key_capturesunit test). For example, a read-access intent (ReadSet) is fuzzy, whereas the resulting view (ViewSet) is specific and concrete, derived by inspecting the current state.Fuzzy keys will be used to implement permissions in the next commit.
Although it’s outside this PoC’s immediate scope, PR #5355 demonstrates how fuzzy keys can enable more fine-grained event filters (designed as receptors).
feat: demonstrate permissioned scenario
I recommend reviewing the previous commit first.
This commit is the latest—and final—step of this PoC, where the instruction lifecycle interacts with the authorizer component, assuming that registrable permissions (#5359) and registrable executables (#5147) have been approved and implemented.
This commit also shows how the tree structures proposed in #5355—where entities share keys—can aid instruction state transitions and batch processing.
Review notes
The concept of flex keys may also be confusing. Flex keys serve to represent the
This(or "self") placeholder (see theflex_key_resolvesunit test). Imagine a common permission that allows users to transfer their own assets. Without flex keys, you would need separate permissions for withdrawing from Alice, Bob, Carol, and so on. In reality, you only need a single permission that allows withdrawal from this authority.README
Executor Modularization & WasmInstruction I/O Simplification PoC
This proof-of-concept explores splitting Hyperledger Iroha’s Executor into modular pieces and slimming down WasmInstruction I/O. The inspiration comes from:
These occupy a key position within tracking issue #5356:
Objective
Can we cleanly separate instruction execution into three roles?
Success means smaller, testable components, fewer FFI round-trips, and clearer extension points.
Repository Structure
Building & Testing
Prerequisites
Guest components
Host tests
cargo test --package host --libcargo test --package host --lib -- tests::instruction_flows --exact --show-outputCompare the test steps to the #5358 state-transition diagram for clarity.
Developer Notes
Host vs. guest, imports vs. exports
wasmtime::component::bindgen!is used on the host side to implement import functions.wit_bindgen::generate!is used on the guest side to implement export functions.Component model trade-offs
unsafeblocks around FFI calls, making future development and maintenance easier:Future developer experience
guest/instruction/src/lib.rsas a reference implementation of smart contracts and trigger executables. It’s intentionally verbose now; later we can introduce syntax sugars.This PoC is experimental and exists solely to test the feasibility of the referenced Iroha issues.