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

Research: wazerod #1475

Closed
wants to merge 3 commits into from
Closed

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jul 2, 2023

Hey I wanted to share with confio and the larger CW community, some research that I have been attempting.

You probably don't want to review this for merge, it's really not ready for that yet.

However, you might, if you find the concept interesting, want to review it.

This template branch is a beginning of an attempt to eliminate dependencies on CGO in WASMD & wasmvm.

As a side effect, it would eliminate the wasmvm repository.

You will also find notes interspersed throughout the code, on places where I think that CGO is harmful.

I'm working on this in a couple of branches, and I'm about to make a merge of one of them to the branch that represents this PR, and you can find more information on the research here:

https://hackmd.io/@xklVAbyuQ9m2DFWqzADekw/B19vvqhuh

current state

Right now this has combined wasmvm and WASMD, and has not yet implemented wazero.

There are a number of open questions that have been outlined in the hackmd above.

foundation of thought

Comment on lines +438 to +442
for k, v := range balances {
dst := make([]types.Coin, len(v))
copy(dst, v)
bal[k] = dst
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism

import (
"fmt"
"reflect"

Check notice

Code scanning / CodeQL

Sensitive package import Note

Certain system packages contain functions which may be a possible source of non-determinism
Comment on lines +48 to +95
go func() {
if useMtx {
defer db.mtx.RUnlock()
}
// Because we use [start, end) for reverse ranges, while btree uses (start, end], we need
// the following variables to handle some reverse iteration conditions ourselves.
var (
skipEqual []byte
abortLessThan []byte
)
visitor := func(i btree.Item) bool {
item := i.(*item)
if skipEqual != nil && bytes.Equal(item.key, skipEqual) {
skipEqual = nil
return true
}
if abortLessThan != nil && bytes.Compare(item.key, abortLessThan) == -1 {
return false
}
select {
case <-ctx.Done():
return false
case ch <- item:
return true
}
}
switch {
case start == nil && end == nil && !reverse:
db.btree.Ascend(visitor)
case start == nil && end == nil && reverse:
db.btree.Descend(visitor)
case end == nil && !reverse:
// must handle this specially, since nil is considered less than anything else
db.btree.AscendGreaterOrEqual(newKey(start), visitor)
case !reverse:
db.btree.AscendRange(newKey(start), newKey(end), visitor)
case end == nil:
// abort after start, since we use [start, end) while btree uses (start, end]
abortLessThan = start
db.btree.Descend(visitor)
default:
// skip end and abort after start, since we use [start, end) while btree uses (start, end]
skipEqual = end
abortLessThan = start
db.btree.DescendLessOrEqual(newKey(end), visitor)
}
close(ch)
}()

Check notice

Code scanning / CodeQL

Spawning a Go routine Note test

Spawning a Go routine may be a possible source of non-determinism
* remove no cgo tests

* repository cleanup

* add test and lint

* cleanup

* wazero-fixes

* all tests pass again

* finish linting

* remove the "builders" folder, it will no longer be needed

* lint docs

* Revert "remove the "builders" folder, it will no longer be needed"

This reverts commit d5d696c.

* format INTEGRATION.md

* bump rust package versions

* keep circleci since we are submitting this upstream for advice / review /ideas

* only run github actions on 1.20

* update ci
@alpe alpe added the spike Demo to showcase an idea. label Jul 3, 2023
@alpe
Copy link
Contributor

alpe commented Jul 3, 2023

Thanks for sharing. Please note that we have very limited capacity and focusing on the sdk 50 upgrade and mesh security.
I have added the spike label to this PR

@faddat
Copy link
Contributor Author

faddat commented Jul 16, 2023

@alpe makes perfect sense, this would certainly be a spike.

@alpe
Copy link
Contributor

alpe commented Oct 18, 2023

@faddat can we close this now?

@faddat
Copy link
Contributor Author

faddat commented Mar 3, 2024

Hey @webmaster128 Thanks for keeping this here as long as you did. I do indeed think it is quite safe to close now.

Makes more sense to check out wasmvm 2.0 instead of reinventing the wheel....

@faddat faddat closed this Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spike Demo to showcase an idea.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants