Fix traces #6127
Fix traces #6127
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ripple/wasmi-host-functions #6127 +/- ##
===========================================================
Coverage 79.5% 79.5%
===========================================================
Files 848 848
Lines 73417 73433 +16
Branches 8401 8402 +1
===========================================================
+ Hits 58387 58400 +13
- Misses 15030 15033 +3
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
S. N. |
| } | ||
|
|
||
| // env test logs don't check severity, so we add it. | ||
| class SuiteJournalSink2 : public SuiteJournalSink |
There was a problem hiding this comment.
You don't need to rewrite this - you can use test::StreamSink.
There was a problem hiding this comment.
No, StreamSink is not from base Sink hierarchy
src/test/app/HostFuncImpl_test.cpp
Outdated
| testGetNFTFlags(); | ||
| testGetNFTTransferFee(); | ||
| testGetNFTSerial(); | ||
| // testGetLedgerSqn(); |
There was a problem hiding this comment.
These should all be uncommented - I assume left over from testing
src/test/app/TestHostFunctions.h
Outdated
| if (!getJournal().active(beast::severities::kTrace)) | ||
| return 0; |
There was a problem hiding this comment.
This will cause consensus issues. You need to return the same value regardless of what the trace level is.
There was a problem hiding this comment.
No, it is perfectly correct - if you writing something you should know that nothing were written because of config/settings.
There was a problem hiding this comment.
On an actual network, different nodes will have different log settings. They will return different values based on those log settings. Depending on the WASM code, this could result in different results in the WASM execution. Different transaction results mean consensus breaks.
src/test/rpc/Subscribe_test.cpp
Outdated
| { | ||
| std::uint32_t idx{0}; | ||
| auto reply = wsc.getMsg(100ms); | ||
| auto reply = wsc.getMsg(200ms); |
There was a problem hiding this comment.
This is an unrelated fix and should be separate (and probably to develop if this is to fix that flaky test)
There was a problem hiding this comment.
You right, it is not actually a fix. Next time it can takes 300ms. Removed
3 participants
High Level Overview of Change
Don't do anything if severity level lower then trace
Type of Change
.gitignore, formatting, dropping support for older tooling)