Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: txn command to simulate #1283

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

feat: txn command to simulate #1283

wants to merge 34 commits into from

Conversation

willemneal
Copy link
Member

@willemneal willemneal commented Apr 15, 2024

Part of #1265
Adds

  • txn simulate - simulate and assemble transaction

Future PR:

  • signer::Stellar trait and a default InMemory implementation needed for txn sign
  • txn sign - sign a given transaction
  • txn inspect
  • txn send - sends a signed transaction to the PRC (not useful without sign)

Left

  • Tests

@willemneal willemneal force-pushed the feat/txn-command branch 6 times, most recently from 2d6ae5b to 0b4cb92 Compare April 16, 2024 17:00
@willemneal willemneal marked this pull request as ready for review April 16, 2024 20:03
@elizabethengelman elizabethengelman mentioned this pull request Apr 25, 2024
5 tasks
@willemneal willemneal force-pushed the feat/txn-command branch 3 times, most recently from 578db4f to 7609f30 Compare May 7, 2024 18:43
@janewang janewang added this to the v21 milestone May 7, 2024
cmd/soroban-cli/src/commands/tx/mod.rs Outdated Show resolved Hide resolved
cmd/soroban-cli/src/commands/tx/mod.rs Outdated Show resolved Hide resolved
cmd/soroban-cli/src/commands/tx/send.rs Outdated Show resolved Hide resolved
cmd/soroban-cli/src/commands/tx/sign.rs Outdated Show resolved Hide resolved
cmd/soroban-cli/src/commands/tx/sign.rs Outdated Show resolved Hide resolved
cmd/soroban-cli/src/commands/tx/xdr.rs Outdated Show resolved Hide resolved
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some commits getting main merged in.

This PR looks like it's still a little ways away from being ready. Left some inline comments.

Would be a good idea to add tests for these commands too, from the outside-in.

@leighmcculloch
Copy link
Member

leighmcculloch commented May 10, 2024

Given the sensitivity of the sign command, I'd like to shift that work into a separate PR, so that we can get the other commands merged sooner, then follow up and take our time with the sign command. There should be litle to no need for the commands to be added together, so I think that'll be fine, but please lmk @willemneal if you think that'd be a problem. cc @janewang

@willemneal
Copy link
Member Author

Friend bot seems to be failing.

@willemneal
Copy link
Member Author

@leighmcculloch I've removed send because it has hard to test without a proper sign command. So there is another PR to add all the remaining besides inspect.

@leighmcculloch leighmcculloch changed the title feat: txn commands to sign, simulate, and send txns feat: txn command to simulate May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants