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] Debug arbitrary variable in programs #892

Merged
merged 10 commits into from
May 9, 2024
Merged

Conversation

iFrostizz
Copy link
Contributor

@iFrostizz iFrostizz commented May 8, 2024

Closes #552, Closes #788
It is currently pretty hard to debug programs. This PR adds logging capabilities to them by the hijacking of the dbg! macro which is optimized away in release builds.

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.

Can I also get a screenshot of what the output looks like? I'm thinking there might be an extra \n in there, but I would have to see the output.

x/programs/rust/wasmlanche-sdk/src/program.rs Outdated Show resolved Hide resolved
x/programs/rust/wasmlanche-sdk/src/state.rs Outdated Show resolved Hide resolved
x/programs/rust/wasmlanche-sdk/src/state.rs Outdated Show resolved Hide resolved
Comment on lines 271 to 274
match result {
0 => Ok(()),
_ => Err(Error::Log),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there should be a return here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you make it panic if the memory reading failed ? I wanted it to stick to the interface of other functions.
Also I think that it should be returning any non-zero value if the call succeeded. The put linker function also always return a 0 wether or not it succeeds which is probably a bug.

Copy link
Contributor Author

@iFrostizz iFrostizz May 8, 2024

Choose a reason for hiding this comment

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

my bad, the error is handled by wasmtime before it is passed back. Should we still panic at this level in the log_bytes function ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The put linker function also always return a 0 wether or not it succeeds which is probably a bug

Probably a bug but it's being rewritten.

Should we still panic at this level in the log_bytes function?

I think we should log a warning. I'm not sure what facilities Go has for conditional compilation, but for production VM instances, the log function shouldn't even be linked.

@iFrostizz
Copy link
Contributor Author

Screenshot 2024-05-08 at 16 56 30

here is a screenshot @richardpringle , I think that it looks fine ?

@iFrostizz iFrostizz requested a review from richardpringle May 8, 2024 14:57
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.

Almost there

x/programs/examples/imports/pstate/pstate.go Show resolved Hide resolved
x/programs/rust/wasmlanche-sdk/src/logging.rs Outdated Show resolved Hide resolved
x/programs/rust/wasmlanche-sdk/src/state.rs Outdated Show resolved Hide resolved
@richardpringle richardpringle merged commit 7494ba6 into main May 9, 2024
26 checks passed
iFrostizz added a commit that referenced this pull request May 14, 2024
* 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>
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>
@richardpringle richardpringle deleted the debug_logs branch August 22, 2024 19:03
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.

[x/programs] Add debug logging host function to wasm. [x/programs] Add support for logger import module
2 participants