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

[x/programs] Fix program calling another program on the Rust side #866

Merged
merged 13 commits into from
Apr 30, 2024

Conversation

iFrostizz
Copy link
Contributor

@iFrostizz iFrostizz commented Apr 26, 2024

The programs calling another contract functionality was fully broken, here are the steps taken

  • Support of a new step parameter called Program, the step ID should be passed the reason is that the previously ID was not decoding the parameter to the right format Normalize all data from call params as a program ID when relevant
  • Make the storage/storage.go temporarly fallback to the duplicated `examples/storage. This is a hack while waiting for @dboehm-avalabs rewrite and lets us read data from the cache
  • Test of the counter on the Rust side to prove this works

@iFrostizz iFrostizz marked this pull request as ready for review April 29, 2024 10:48
iFrostizz and others added 3 commits April 29, 2024 21:52
* add cache

* implement cache reads

* actually cache reads

* implement cache delete and handle borsch errors

* use only one cache map

* remove flushed bool

* root of transactions by stateless merkledb

* make root generation a function

* preallocate memory for merkle array and consumebytes flag

* add <*.code-workspace> to .gitignore and remove it from git commit

* move root generation func to merkle package, tx root by items of [txID + result]

* rebase & blk marshal/unmarshal & merkleroot to ids.ID

* write benches for the merkle package

* use crypto/rand, fix var name, report allocs

* put the 10k bench back

* pass config by parameter

* happy clippy

* borrow V

* add TODO

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "put the 10k bench back"

This reverts commit 058d7e7.

* Revert "use crypto/rand, fix var name, report allocs"

This reverts commit 214005b.

* Revert "write benches for the merkle package"

This reverts commit 07993bf.

* Revert "rebase & blk marshal/unmarshal & merkleroot to ids.ID"

This reverts commit 7442836.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "add <*.code-workspace> to .gitignore and remove it from git commit"

This reverts commit ce00289.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* merge main!

* merge imports

---------

Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>
* macro skeleton

* write macro for ffi-safe bindings

* avoid panicking todo!

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* remove unused arms

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
@iFrostizz iFrostizz self-assigned this Apr 30, 2024
Copy link
Contributor

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

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

Well done

if err != nil {
return nil, err
if strings.HasPrefix(stepIdStr, "step_") {
programIdStr, ok := c.programIDStrMap[stepIdStr]
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔, I think we need to be weary that this is supposed to be more generic than it is. I think this was supposed to useable for any result of a previous step, though, looking at the code, that does not appear to be the case.

No change necessary here, but we'll have to keep this in mind when we eventually go to improve the simulator.

Comment on lines +69 to +74
return nil, false, nil
}
if err != nil {
return nil, false, err
}
return v, true, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

😳

@@ -15,7 +15,8 @@ import (
)

const (
programPrefix = 0x0
programPrefix = 0x0
ProgramChunks uint16 = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this being used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, it was for mimicking a block of the vm/storage/storage.go file but it ended up being removed apparently, will fix in a followup!

mod tests {
use simulator::{Endpoint, Key, Param, Plan, Require, ResultAssertion, Step};

const PROGRAM_PATH: &str = env!("PROGRAM_PATH");
Copy link
Contributor

Choose a reason for hiding this comment

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

😞... I really don't like that this is how we specify the .wasm path (and I'm the one that did this)... if you have any better ideas, please share in a new issue.

endpoint: Endpoint::Execute,
method: "initialize_address".into(),
max_units: 1000000,
params: vec![counter1_id.into(), alice_key.clone()],
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to clone alice_key here... but I'll allow it

Comment on lines +111 to +114
// run plan
let plan_responses = simulator.run_plan(&plan).unwrap();

// ensure no errors
Copy link
Contributor

Choose a reason for hiding this comment

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

I think both these steps are super clear, no need to add the extra comments.

.next()
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

This is the way testing should always be done! 🥇

@richardpringle richardpringle merged commit 4ef8b2a into ava-labs:main Apr 30, 2024
34 checks passed
iFrostizz added a commit that referenced this pull request May 1, 2024
* support program passing in param

* fix inter-program interaction

* update fixture

* keep using `SmartPtr` for now

* remove the need of a new Param type

* lint

* remove param

* fix nits

* remove whitespace

* Cached state values in the program state (#840)

* add cache

* implement cache reads

* actually cache reads

* implement cache delete and handle borsch errors

* use only one cache map

* remove flushed bool

* root of transactions by stateless merkledb

* make root generation a function

* preallocate memory for merkle array and consumebytes flag

* add <*.code-workspace> to .gitignore and remove it from git commit

* move root generation func to merkle package, tx root by items of [txID + result]

* rebase & blk marshal/unmarshal & merkleroot to ids.ID

* write benches for the merkle package

* use crypto/rand, fix var name, report allocs

* put the 10k bench back

* pass config by parameter

* happy clippy

* borrow V

* add TODO

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "put the 10k bench back"

This reverts commit 058d7e7.

* Revert "use crypto/rand, fix var name, report allocs"

This reverts commit 214005b.

* Revert "write benches for the merkle package"

This reverts commit 07993bf.

* Revert "rebase & blk marshal/unmarshal & merkleroot to ids.ID"

This reverts commit 7442836.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "add <*.code-workspace> to .gitignore and remove it from git commit"

This reverts commit ce00289.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* merge main!

* merge imports

---------

Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* [x/programs] safe wrapper around C ffi interface (#869)

* macro skeleton

* write macro for ffi-safe bindings

* avoid panicking todo!

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* remove unused arms

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>
iFrostizz added a commit that referenced this pull request May 6, 2024
* support program passing in param

* fix inter-program interaction

* update fixture

* keep using `SmartPtr` for now

* remove the need of a new Param type

* lint

* remove param

* fix nits

* remove whitespace

* Cached state values in the program state (#840)

* add cache

* implement cache reads

* actually cache reads

* implement cache delete and handle borsch errors

* use only one cache map

* remove flushed bool

* root of transactions by stateless merkledb

* make root generation a function

* preallocate memory for merkle array and consumebytes flag

* add <*.code-workspace> to .gitignore and remove it from git commit

* move root generation func to merkle package, tx root by items of [txID + result]

* rebase & blk marshal/unmarshal & merkleroot to ids.ID

* write benches for the merkle package

* use crypto/rand, fix var name, report allocs

* put the 10k bench back

* pass config by parameter

* happy clippy

* borrow V

* add TODO

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "put the 10k bench back"

This reverts commit 058d7e7.

* Revert "use crypto/rand, fix var name, report allocs"

This reverts commit 214005b.

* Revert "write benches for the merkle package"

This reverts commit 07993bf.

* Revert "rebase & blk marshal/unmarshal & merkleroot to ids.ID"

This reverts commit 7442836.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "add <*.code-workspace> to .gitignore and remove it from git commit"

This reverts commit ce00289.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* merge main!

* merge imports

---------

Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* [x/programs] safe wrapper around C ffi interface (#869)

* macro skeleton

* write macro for ffi-safe bindings

* avoid panicking todo!

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* remove unused arms

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>
richardpringle added a commit that referenced this pull request May 14, 2024
…rough plan run (#884)

* fix logging

* fix nits

* set log level to error in the simulator client & write logs to Stderr

* [x/programs] Fix program calling another program on the Rust side (#866)

* support program passing in param

* fix inter-program interaction

* update fixture

* keep using `SmartPtr` for now

* remove the need of a new Param type

* lint

* remove param

* fix nits

* remove whitespace

* Cached state values in the program state (#840)

* add cache

* implement cache reads

* actually cache reads

* implement cache delete and handle borsch errors

* use only one cache map

* remove flushed bool

* root of transactions by stateless merkledb

* make root generation a function

* preallocate memory for merkle array and consumebytes flag

* add <*.code-workspace> to .gitignore and remove it from git commit

* move root generation func to merkle package, tx root by items of [txID + result]

* rebase & blk marshal/unmarshal & merkleroot to ids.ID

* write benches for the merkle package

* use crypto/rand, fix var name, report allocs

* put the 10k bench back

* pass config by parameter

* happy clippy

* borrow V

* add TODO

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "put the 10k bench back"

This reverts commit 058d7e7.

* Revert "use crypto/rand, fix var name, report allocs"

This reverts commit 214005b.

* Revert "write benches for the merkle package"

This reverts commit 07993bf.

* Revert "rebase & blk marshal/unmarshal & merkleroot to ids.ID"

This reverts commit 7442836.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "add <*.code-workspace> to .gitignore and remove it from git commit"

This reverts commit ce00289.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* Revert "pass config by parameter"

This reverts commit 4aec589.

* Revert "move root generation func to merkle package, tx root by items of [txID + result]"

This reverts commit e551960.

* Revert "preallocate memory for merkle array and consumebytes flag"

This reverts commit 68e49b6.

* Revert "make root generation a function"

This reverts commit aa44f97.

* merge main!

* merge imports

---------

Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* [x/programs] safe wrapper around C ffi interface (#869)

* macro skeleton

* write macro for ffi-safe bindings

* avoid panicking todo!

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* remove unused arms

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* stdin subcommand

* use argparse

* iteratively parse subcommand approach

* use a real shell parser

* nil pointer deref

* enable steps reading from stdin

* run steps sequentially

* sequential plan

* reduce changes

* read plan from file

* fix hanging on stdout result

* fix forgotten stdout

* kill process to avoid EOF

* add failing assertion test and fix unreachable in simulator

* apply review suggestions

* fix last nits

* add license

* apply commit suggestions

* apply review suggestions

* own the data streams instead

* stop using double pointers

* Make `SimulatorChild` generic

* simplify stdin run command interaction

* fix nits

* remove useless test

* address most comments

* fix handle getting

* pass db to `Run`

* Make delete return the old data (#890)

* Make delete return the old data

* Add comments about enum

* Make `#[public]` ffi functions private and unsafe (#893)

* Make `#[public]` ffi functions private and unsafe

* Fix macro warning through type swap

* Encapsulate mutability in the macro

* [x/programs] Debug arbitrary variable in programs (#892)

* add log

* reb

* implement linker function

* remove debug leftover

* remove host file

* remove unused explicit type

* apply review suggestions

* address review

* Remove `Log` error from `state::Error`

Signed-off-by: Richard Pringle <rpring9@gmail.com>

---------

Signed-off-by: Richard Pringle <rpring9@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* Make Rust crate naming conventions consistent (#895)

* Make all #[public] functions allocate the response (#896)

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
Signed-off-by: Richard Pringle <rpring9@gmail.com>
Co-authored-by: bianyuanop <chen.me.nan@gmail.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants