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

Integration of the orchestrator API and authentication based on payload signature #221

Merged
merged 29 commits into from
Apr 9, 2020

Conversation

antho1404
Copy link
Member

@antho1404 antho1404 commented Apr 5, 2020

  • Add new .mesgrc config to store user's mnemonic and config for the engine
  • Add user's public key to the engines authorized accounts
  • Create orchestrator library based on the proto definition
  • Use orchestrator lib for the runner
  • Use orchestrator lib in the CLI
  • Sign all requests from the CLI with user's mnemonic
  • Add token to the user from the engine's account

Test

  • Use the branch test/cli-mnemonic that includes a runner that doesn't run in docker (and has access to the library not yet released)
  • Build the engine image make docker-dev
  • Select a service
  • npm install path-to-the-js-sdk/packages/service
  • `./bin/run service:dev path-to-service --version local

@github-actions
Copy link

github-actions bot commented Apr 6, 2020

Please update the CHANGELOG of the associated library for your Pull Request to be accepted

@antho1404 antho1404 changed the title Feature/cli mnemonic orchestrator api Apr 7, 2020
})
}
}
])
await tasks.run()

cli.styledJSON(decode(execution.outputs))
styledJSON(decode(execution.outputs))
process.exit(0)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to exit the process without having to cancel the log and have the warning from GRPC.

Copy link
Member

Choose a reason for hiding this comment

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

it will be nicer to catch the error and not displaying it in order to really remove the stream from the server.
But ok as it's local dev only

// https://github.com/forbole/big-dipper/blob/master/imports/startup/server/util.js
const pubKey = (mnemonic: string): string => {
const publicKey = Account.deriveMnemonic(mnemonic).publicKey
const pubkeyAminoPrefix = Buffer.from('EB5AE98721', 'hex')
Copy link
Member Author

Choose a reason for hiding this comment

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

More information here and here

packages/cli/src/utils/environment-tasks.ts Outdated Show resolved Hide resolved
super(endpoint, 'Event')
}

public stream(request: StreamRequest, signature: string): grpc.ClientReadableStream<Type.mesg.types.IEvent> {
Copy link
Member Author

Choose a reason for hiding this comment

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

StreamRequest and all attributes of these functions are here in order to "normalize" the data between the serialization in json/amino and the one in proto.
The signature is based on the json serialization while the payload is sent in proto which is really confusing.


// TODO: To improve using the types but if include the type, the definition is still included in the js
export const Status: { [key: string]: Type.mesg.types.Status } = {
Unknown: 0, // Type.mesg.types.Status.Unknown,
Copy link
Member Author

Choose a reason for hiding this comment

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

Read the todo, there is some weird behavior with enums in typescript, the file stays included when compiled even if it doesn't generate any js... maybe a bug in typescript

@antho1404 antho1404 marked this pull request as ready for review April 8, 2020 15:10
@antho1404 antho1404 changed the title orchestrator api Integration of the orchestrator API and authentication based on payload signature Apr 8, 2020
packages/api/src/transaction.ts Show resolved Hide resolved
})
}
}
])
await tasks.run()

cli.styledJSON(decode(execution.outputs))
styledJSON(decode(execution.outputs))
process.exit(0)
Copy link
Member

Choose a reason for hiding this comment

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

it will be nicer to catch the error and not displaying it in order to really remove the stream from the server.
But ok as it's local dev only

packages/cli/src/utils/environment-tasks.ts Outdated Show resolved Hide resolved
const userBalance = userAccount.coins.find(x => x.denom === 'atto')
if (userBalance && parseInt(userBalance.amount, 10) > 1000) return
const transferMsg = api.account.transferMsg(engineAccount.address, userAccount.address, [{
amount: 1e18.toString(),
Copy link
Member

Choose a reason for hiding this comment

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

is it enough? 1e18 is 1 actual coin.
in the engine e2e, it transfers 100,000,000 x 10^18 atto

Copy link
Member Author

Choose a reason for hiding this comment

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

Can definitely be increased, just here the engine will do all the processing, the CLI should just use that to deploy. So the question is is it enough to deploy multiple processes and services.
I think 100k is really a lot just for deployment

packages/service/CHANGELOG.md Outdated Show resolved Hide resolved
packages/service/src/index.ts Show resolved Hide resolved
Copy link
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

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

https://github.com/mesg-foundation/js-sdk/pull/221/files#diff-7061e640ee22641a8cbd0d23cd1903efR135

const logs = res.split('\n').map(x => {
  try {
    return JSON.parse(x)
  } catch (e) {
-    return null
+    return reject(e)
  }
}).filter(x => x)

@NicolasMahe NicolasMahe added this to the next milestone Apr 9, 2020
@antho1404 antho1404 merged commit 30d8454 into master Apr 9, 2020
@antho1404 antho1404 deleted the feature/cli-mnemonic branch April 9, 2020 07:47
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.

2 participants