fix rosettaCombinePayload

[qwadratic]

because actually it is a string

чт, 2 мая 2024 г. в 19:05, Martin Minkov @.***>:

@.**** commented on this pull request.

In src/mina-signer/src/rosetta.ts https://github.com/o1-labs/o1js/pull/1633#discussion_r1587988536:

@@ -282,7 +282,7 @@ function rosettaTransactionToSignedCommand({ }

type UnsignedPayload = {

• unsigned_transaction: UnsignedTransaction;
• unsigned_transaction: string;

What is the issue with unsigned_transaction being of type UnsignedTransaction? Is there no way to keep it typed here?

— Reply to this email directly, view it on GitHub https://github.com/o1-labs/o1js/pull/1633#pullrequestreview-2036338293, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLV5LRVICH4QDBBWAI4H3TZAJW4BAVCNFSM6AAAAABHDKQ3AWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZWGMZTQMRZGM . You are receiving this because you authored the thread.Message ID: @.***>

[MartinMinkov]

Seems like there's some tests to fix: https://github.com/o1-labs/o1js/actions/runs/8926010581/job/24521454236?pr=1633

[qwadratic]

@MartinMinkov can you please run tests again? why do we need aprroval to run the workflow?

[qwadratic]

because I get Preset ts-jest/presets/js-with-ts not found. when trying to run tests locally

[mitschabaude]

why do we need aprroval to run the workflow?

If we allowed all external contributors to run workflows, that could lead to exploits because workflows can access CI secrets

[mitschabaude]

because I get Preset ts-jest/presets/js-with-ts not found. when trying to run tests locally

@qwadratic does this work?

git submodule update --init --recursive
npm i
npm run build
./jest src/mina-signer/tests/rosetta.test.ts

if no, where does it fail?

[qwadratic]

Fixed!

[qwadratic]

@mitschabaude @MartinMinkov what can we do with failing benchmark check? this PR doesn't touch o1js

[mitschabaude]

@mitschabaude @MartinMinkov what can we do with failing benchmark check? this PR doesn't touch o1js

You can ignore that failure @qwadratic, it just doesn't work for external PRs

Looks like this is good to go once @MartinMinkov approves!

[qwadratic]

My example code just don't work with current implementation

The integration code should be as simple as possible, thats the intent

[qwadratic]

the typing here was just wrong, the rosetta response has string typed unsigned_transaction (which is json payload for signing)

I fixed it and now its right