Skip to content

Conversation

@kaimast
Copy link
Contributor

@kaimast kaimast commented Jul 23, 2025

Replaces PR #3606 and fixes issue #3361.

Reproducing the Bug

On staging, run test network using ./devnet.sh. When you query the bonded validator mapping, it should give you the incorrectly formatted output.

~> curl http://localhost:3030/testnet/program/credits.aleo/mapping/bonded/$VALIDATOR_ADDR
"{\n  validator: $VALIDATOR_ADDR ,\n  microcredits: 187500058346732u64\n}"

(here $VALIDATOR_ADDR is a bonded address, that can be retrieved from the logs)

Testing

On the PR branch (fix/value-json) run another test network and execute the above command again. Now you will get an output with the correct formatting.

~> curl http://localhost:3030/testnet/program/credits.aleo/mapping/bonded/$VALIDATOR_ADDR
{
  "validator": "$VALIDATOR_ADDR",
  "microcredits": "187500297811718u64"
}

@kaimast kaimast marked this pull request as ready for review July 23, 2025 22:00
@raychu86
Copy link
Collaborator

Would the value_to_json and mapping_to_json functions be more useful in snarkVM?

@kaimast
Copy link
Contributor Author

kaimast commented Jul 25, 2025

Would the value_to_json and mapping_to_json functions be more useful in snarkVM?

Yeah, that would make sense if you think there are other use cases for it. Maybe we should even have something like Value::into_json().

Initially, I was also wondering if it makes more sense to just fix the serialization of Value, but I wasn't sure if that would break something else, and the previous PR was already close to finished.

@vicsn
Copy link
Collaborator

vicsn commented Jul 25, 2025

Thank you for picking this up. @kaimast which other endpoints exhibit this issue?

Based on Kai's answer, @HarukaMa @miazn @iamalwaysuncomfortable are you ok if we change the serialization in a backwards-incompatible way? Or will this break existing tooling to an unbearable degree?

@HarukaMa
Copy link
Contributor

I don't think I'm relying on this specific api behavior so I'm not affected.

However, I think the old format is potentially useful if people are manually executing transactions via snarkos developer execute, as the old output format is directly usable as inputs (especially records, not sure if it's applicable here). There would be no other issues if the execute cli can also consume valid json.

@kaimast
Copy link
Contributor Author

kaimast commented Jul 26, 2025

However, I think the old format is potentially useful if people are manually executing transactions via snarkos developer execute, as the old output format is directly usable as inputs (especially records, not sure if it's applicable here). There would be no other issues if the execute cli can also consume valid json.

Thank you for pointing this out. I will test that developer execute still works as expected and fix it, if needed.

@vicsn vicsn marked this pull request as draft August 7, 2025 16:58
@kaimast
Copy link
Contributor Author

kaimast commented Sep 24, 2025

@vicsn this issue is tagged for Q4, but I am not sure what we decided on with regard to changing the API behavior. I think it should change, but we could add a flag to force the old behavior.

Also, we haven't decided if the value-to-JSON functionality should reside in snarkVM or remain in snarkOS (as implemented by this PR). The former would require more discussion, as we are trying to avoid API breakage in snarkVM.

@vicsn
Copy link
Collaborator

vicsn commented Oct 10, 2025

This is not priority at the moment but adding this gem here for shared context: ProvableHQ/snarkVM#2404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants