bring Rosetta dependencies up to date #261
Conversation
pro-wh
requested review from
abukosek,
kostko,
peterjgilbert and
tjanez
as code owners
okay |
pro-wh
force-pushed
the
pro-wh/feature/update
branch
from
a059351 to
365335b
Compare
|
oh no load_env something is chopping off the |
pro-wh
force-pushed
the
pro-wh/feature/update
branch
2 times, most recently
from
fef15d9 to
9e8d7cb
Compare
|
in the meantime, I'm going to use our server's own /network/list to populate an env var directly in JSON format. this also makes us not need to add a dependency on jq. |
There was a problem hiding this comment.
Great work, @pro-wh. Really easy to review and navigate thanks to nicely breaken down commits!
I only have some minor inline nits and 3 suggestions for the git commit organization:
-
I would squash all 3 "/construction/*: update account_identifier" commits into a single one.
-
I would also squash all 3 "new arguments to NewServer" commits into a single one.
-
Can you squash the "add changelog" commit into the first commit ("bump rosetta and oasis-core versions")?
|
ok on the squashing. |
This allows Rosetta to learn about the test entity's starting balance automatically.
pro-wh
force-pushed
the
pro-wh/feature/update
branch
from
80263e4 to
e28940d
Compare
pro-wh
force-pushed
the
pro-wh/feature/update
branch
from
e28940d to
a2301cd
Compare
|
oh hey, shoutout to @songchenwen who did similar work in #214 |
| sender_amount_fee = 0 - {{amount_fee}}; | ||
| available_amount = {{sender.balance.value}} - {{amount_fee}}; | ||
| recipient_amount = random_number({"minimum": "1", "maximum": {{available_amount}}}); | ||
| print_message({"recipient_amount": {{recipient_amount}}}); |
There was a problem hiding this comment.
Is this print also for debugging or should be there?
There was a problem hiding this comment.
I have this one here to match an equivalent print_message in the Ethereum workflow:
https://github.com/coinbase/rosetta-ethereum/blob/56328e8f0855edcaf6385a7d850142d8ace3b849/rosetta-cli-conf/testnet/ethereum.ros#L59
I'm fine with removing it, since we don't have anyone or that specifically looks at this printout.
But the argument for leaving it in is (1) minimizing how much we diverge from an officially maintained upstream workflow, which should make maintenance easier and (2) hoping that it's useful debugging information to the extent that upstream left it in, which maybe we'll need too at some point
|
thanks for reviewing, merging as is. |
I'm just starting to read through the various changelogs and stuff, but here's a placeholder PR to indicate that we're looking to upgrade to the latest of the Rosetta SDK and cli
Obsoletes: #214, #253, #254, #255.