Skip to content

Conversation

@samuelWilliams99
Copy link
Contributor

When operating with a remote CLI, I noticed a few issues:

  • Balancing transactions required the signing keys file to be on the remote machine, but they weren't written until submission
  • The txs folder is always created on the local machine, whereas, if we're running remote, it needs to be on the remote machine
  • Various files and directories are created in non-optimum places

@samuelWilliams99 samuelWilliams99 added the bug Something isn't working label Feb 17, 2022
@samuelWilliams99 samuelWilliams99 linked an issue Feb 17, 2022 that may be closed by this pull request
Copy link
Collaborator

@szg251 szg251 left a comment

Choose a reason for hiding this comment

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

LGTM.
I assume you have already checked with a real remote server.
Might not really worth the effort, but I think we could extend the MockContract testing framework so that it checks for files on cli calls, so we don't run into the same kind of trouble after a refactoring.

@samuelWilliams99
Copy link
Contributor Author

Yep I've tested on real remote. I havent however tested on local, as I dont have that environment readily available. I don't expect these changes to even remotely affect Local, but it might be worth checking if you know of a quick way to do so?

@szg251
Copy link
Collaborator

szg251 commented Feb 22, 2022

I'll just check them myself

@samuelWilliams99
Copy link
Contributor Author

As for testing, how do you mean? Would we keep track of a mock file system and parse the cli calls to ensure they refer to existing files?

@szg251
Copy link
Collaborator

szg251 commented Feb 22, 2022

We already have that mock file system here: https://github.com/mlabs-haskell/bot-plutus-interface/blob/master/test/Spec/MockContract.hs#L198 but it doesn't handle uploadDir yet
cli call parsing could get a bit tedious

@szg251
Copy link
Collaborator

szg251 commented Feb 22, 2022

@samuelWilliams99 Did a few tests with the examples on the local environment, it looks good.

@samuelWilliams99
Copy link
Contributor Author

cli call parsing could get a bit tedious

Depends, we dont need to parse the whole thing. theres a set of specific flags that are followed by a path, we can just look for those then grab the next thing up to a space (or quoted)

@samuelWilliams99
Copy link
Contributor Author

I think the tests would probably go best with the tx delete changes, as we can generalise to ensure that works as well

@samuelWilliams99
Copy link
Contributor Author

Or, at the very least, a new ticket. This one is a bug fix

@samuelWilliams99
Copy link
Contributor Author

Going to handle the mock file system improvements in #65

@samuelWilliams99 samuelWilliams99 merged commit 7d78b3d into master Feb 22, 2022
@samuelWilliams99 samuelWilliams99 deleted the sam/fix-remote-cli branch February 22, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote has bugs

3 participants