Feat/wasm upgrade #49
Conversation
…, added new block for re-enabling of IBCs
…Cost and canonicalizeCost
fragwuerdig
requested review from
edk208,
faddat,
nghuyenthevinh2000,
catShaark,
ZaradarBH and
vuong177
ZaradarBH
added
enhancement
| @@ -8,7 +8,7 @@ import ( | |||
|
|
|||
| // Constant gas parameters | |||
| const ( | |||
| GasMultiplier = uint64(100) // Please note that all gas prices returned to the wasmVM engine should have this multiplied | |||
| GasMultiplier = uint64(140_000_000) // Please note that all gas prices returned to the wasmVM engine should have this multiplied | |||
There was a problem hiding this comment.
Please have a look at the wasmd reference implementation. The wasmvm gas calculations made a major step in the upgrade from 0.16 -> 1.0.0. wasmd v0.27.0 is the version that fully incorporates the new wasmvm. The final gas multiplier was set to 140,000,000 to convert sdk gas to wasmer gas and vice versa.
| }, | ||
| "recursion 1, no work": { | ||
| gasLimit: 400_000, | ||
| msg: Recurse{ | ||
| Depth: 1, | ||
| }, | ||
| expectedGas: 2*GasNoWork + GasReturnUnhashed, | ||
| expectedGas: 0x150f8, |
There was a problem hiding this comment.
it's changed the test logic. Can you tell me where this change come from?
There was a problem hiding this comment.
The new gas calculation strategy in wasmvm leads to different values in the expected gas. This also happened in the wasmd update to v0.27. They needed to tinker with the expected test values too. I simply took the values that the test would expect.
I couldn't take the test values from wasmd. Because, the thing with terrad and wasmd is that the gas cost for emitted events from a contract call are inherently different (terrad charges per byte, while wasmd charges per event). That's why I kinda figured the expected test values are more or less regression reassurances for updates that come after this one.
|
I am comparing your work to this branch of wasmd: https://github.com/CosmWasm/wasmd/tree/v0.28.0/x/wasm Originally, I think terra classic team just copy x/wasm into their repo IBC feature will be missing from our wasm module version which I think is a really big hit for cross - chain smart contract |
|
@fragwuerdig Trying to merge this one will override some changes in current main. I make a branch from main here: #67 |
|
@nghuyenthevinh2000: Should we close this one in favor of #67? |
Yes, I think so. #67 will not override logic in main branch when merge |
|
Closed in favor of #67 |
Summary of changes
This is the WASM upgrade from the PR over here I cherry-picked the commits and based them on v1.0.4 release.
There is one odd thing I have. In order to complete go mod tidy I needed to remove the line from the replace section in go.mod
github.com/tecbot/gorocksdb => github.com/cosmos/gorocksdb v1.2.0Otherwise, if I don't do it, the go mod tidy won't complete, rendereing go.sum useless and breaking the build. Can somebody please review on that one?
Report of required housekeeping
(FOR ADMIN) Before merging