-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add tezos signer as a part of the FF stack for the tezos flow #275
Conversation
Signed-off-by: Dzianis Andreyenka <[email protected]>
Signed-off-by: Dzianis Andreyenka <[email protected]>
Signed-off-by: Dzianis Andreyenka <[email protected]>
Signed-off-by: Dzianis Andreyenka <[email protected]>
addresses := "" | ||
for i, member := range p.stack.Members { | ||
account := member.Account.(*ethereum.Account) | ||
addresses = addresses + account.Address | ||
if i+1 < len(p.stack.Members) { | ||
addresses = addresses + "," | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a reason this was removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused code
pkg/types/manifest.go
Outdated
@@ -32,6 +32,7 @@ type VersionManifest struct { | |||
TokensERC1155 *ManifestEntry `json:"tokens-erc1155"` | |||
TokensERC20ERC721 *ManifestEntry `json:"tokens-erc20-erc721"` | |||
Signer *ManifestEntry `json:"signer"` | |||
TezosSigner *ManifestEntry `json:"tezossigner"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the TezosSigner needs to go in the manifest. See comments in the FireFly Core PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are your thoughts on running signer service as part of the ff stack without using manifest?
In our case, this is a service that should start by default and work for signing transactions (as ff-signer).
An advanced user will be able to replace it with the new one with a configured key management system that suits his goals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. The FireFly CLI starts up several things that are not in the manifest, including blockchain nodes, running fabric-tools, etc. I think it can run the Tezos signer but we don't need to have it in the manifest.
The only reason the other signer entry is in the manifest is because it is a firefly-
GitHub repo and part of the project itself, so we like to test and publish specific versions together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Pointed the image explicitly instead of using FF manifest.
Thanks @nguyer for your opinion!
Signed-off-by: Dzianis Andreyenka <[email protected]>
1bdc889
to
58f681a
Compare
Signed-off-by: Dzianis Andreyenka <[email protected]>
No description provided.