-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
@@ -14,6 +14,91 @@ export function utf8Decode(data: Uint8Array): string | undefined { | |||
} | |||
} | |||
|
|||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars | |||
function parseAlgosdkV2SimulateResponse(obj: any): any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note, SimulateResponse
objects encoded via JSON.stringify(simulateResponse.get_obj_for_encoding())
using <.3.0 js-algorand-sdk
results in a json string where addresses and byte fields are encoded into base64. While >3.0 js-algorand-sdk
clients will fail when attempting to algosdk.decodeJSON(jsonEncodedSimualteResponseString, SimulateResponse)
-> since it doesn't expect base64 strings for address fields and keys like gh and apaa. So most of the compatibility issues seems to be on the encoded transaction objects. Wasn't clear if v2 encoded SimulateResponses are compatible with v3 decoders based on the note on related topic in migration guide https://github.com/algorand/js-algorand-sdk/blob/develop/v2_TO_v3_MIGRATION_GUIDE.md#object-encoding-and-decoding.
If it's a breaking change and its expected behaviour on js sdk side, we will probably keep the implementation here as is to have backwards compatibility with projects that encoded simulate traces with v2 sdk. Otherwise I can submit it as issue on js-sdk repo and keep this method temporarily until resolved on js-sdk 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that algosdk v2 would produce base64-encoded raw address bytes when asked to convert a simulate response to JSON like that. I consider that a bug in v2, since that's not the way algod produces a JSON simulate response.
I believe simulate responses are the only model type that's affected by this issue, since that's the only response type in v2 which unpacks into a typed response class and which comes from a msgpack-encoded response (meaning the addresses are internally stored as byte arrays, not strings).
While this has been fixed in v3, you're correct in pointing out that there's a potential compatibility issue if you're using an artifact that was produced using the v2 SDK.
From my perspective, adding code to this repo which handles compatibility between the two versions seems like a reasonable approach.
* chore: adding option to treat null under sourcemap-locations as valid 'ignore' cases * chore: npm audit * chore: adding npm prepare script for installation via github * feat: add puya debug support (WIP) * chore: support for passing file contents for sourcemaps; puya/teal edge cases (#1) * chore: hiding locals on non puya sourcemaps; adding puya and teal example * chore: no tmpl vars offset edge case fix * chore: rel path for example on puya + teal * chore: workaround on skipping syntentic refs that does not require async file exists * feat: support direct file or encoded content for sourcemaps * refactor: make programSourcesDescription an object instead of JSON * chore: adding more edge cases * chore(src/common): Add program source entry file and type Added 'ProgramSourceEntryFile' and 'ProgramSourceEntry' to the common module. These types are used for parsing program source files. * chore: add support for 'null' sourcemap paths when user prefers ignores * chore: add mocharc file for global timeouts Also fixing a few tests * fix: move program forward call after inner transaction processing * chore: further refine puya example with third parties * feat: integrating Locals with existing expand avm methods * chore: adding puya unit test; moving process unit into program replay --------- Co-authored-by: Daniel McGregor <[email protected]> * fix: step backwards on puya sourcemaps * refactor: use different interface for CallStackFrame vs TraceReplayFrame capture lastIndex for when needing to walk backward in a program remove _ expandedAvmValueCache and instead handle the 'named' and 'indexed' filters correctly * feat: support evaluation of Puya variables * chore: reduce diff * chore: bumping to algosdkv3; adding puya-ts example; extra unit test; * fix: backwards compatibility with SimulateResponse encoded via algosdkv2 * fix: update unit tests to match latest examples * fix: hiding locals on non puya frontend sourcemap paths * chore: transferring advanced examples to extension repo * chore: apply suggestions from code review Co-authored-by: Neil Campbell <[email protected]> --------- Co-authored-by: Daniel McGregor <[email protected]> Co-authored-by: Neil Campbell <[email protected]>
@gmalouf i can see your comment in an email on a typo in package-lock where algokit-utils-ts was included - for some reason its not displayed on PR and wasn't visible on the git diff when i was opening the pull request. Regardless though just did an amend commit which removes this - the avm debugger does not rely on algokit utils in any way (was just a typo) 👍 |
@gmalouf will you guys publish on NPM too? |
Proposed changes