-
Notifications
You must be signed in to change notification settings - Fork 99
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
xGov 178: SIWA - Sign In With Algorand #178
Conversation
If your proposal is ready, please update the status to |
This proposal overlaps with on-going efforts. ARC31, ARC60 and the FIDO2/passkeys on-going work for authentication. Also, there's already existing implementations and actual implementations on those. This proposal doesn't seem to add much for the current state of the ecosystem standards development |
IMHO, sign in with Algorand (SIWA) or any other attempt to replicate ERC-4361 for Algorand should not be attempted as implementation or tooling libraries but instead should be an ARC contribution. Also it is notable that ARCs and ERCs are not paid subjects. |
SIWA Deliverables (6/18/2024): SIWA-NPM: https://www.npmjs.com/package/@avmkit/siwa CSR-Template-deployed: https://algostack.vercel.app SSR-Template-deployed: https://algostack-ssr.vercel.app SIWA-website-https://siwa.org SIWA-template-marketplace: https://siwa.org/templates |
@headline-design I have concerns regarding your delivery.
Looking forward for your reply. Thanks! |
@headline-design I'm a bit confused, I agree with @emg110 and @ehanoc on this one I look forward to understanding this better, is there a thread that has more context about this proposal? The original proposal references docs.login.xyz which already outlined the Next.js integration path. I can't seem to find anywhere in the proposal that mentions the dependency on a dedicated service. I also don't see any guides on how to use the main deliverable, the
Few questions: What was the decision process around creating a bespoke template and hosting platform for SIWA? The template and service includes other opinionated decisions which don't seem directly related to SIWA. What was the decision behind not publishing the required Auth.js provider package as a part of the deliverables (similar to WAGMI+siwe and what appeared to be relayed in the forum)? Another question is why does the integration path require registering with a centralized application? SIWA Dashboard does not seem functional, currently throws 500 when using Pera on Step 1 using the guide illustrated here. I was able to bypass the error to explore the product but the connect experience seems broken currently. Related: With all due respect, it seems like the SIWA scope has creeped to be an alternative to |
Hi @headline-design, the Foundation is working on reviewing the technical deliverables for this xGov grant. In order for this review to proceed, please provide the following:
We look forward to your response! |
@ehanoc can you share with @headline-design the paper we wrote on BIP32-Ed25519 Hierarchical Deterministic Keys over a Non-linear Keyspace, to give them an example of what I expect. A security description of the protocol is needed. |
It's here: https://github.com/algorandfoundation/bip32-ed25519-addendum |
Which document in that repo are you referring too? |
Hey folks, this one https://github.com/algorandfoundation/bip32-ed25519-addendum/blob/main/paper.tex |
@headline-design I took a look at the source code of SIWA and it contains JSDoc comments that seem odd, referring to the Ethers library and EIPs, e.g.,
A GitHub global search of one of these comments yields the original SIWE library along with several forks, including your https://github.com/headline-design/avmkit repository's SIWA package: https://github.com/search?q=%22This+function+can+be+used+to+retrieve+an+EIP-4361+formated+message%22&type=code Comparing the two libraries, it is clear that SIWA is ostensibly a fork of SIWE. But there is no mention of SIWE anywhere in the source code. The client.ts file, for example, is virtually identical: https://www.diffchecker.com/iGSuqsk2/ The SIWE library has both Apache 2.0 and MIT licenses, which both require clear attribution in forked versions. The lack of attribution could be interpreted as a violation of the dual license. Even if it isn't, there are obviously ethical considerations since you are seeking a grant for what is largely someone else's work. |
As an update, the following matters still need to be addressed by the headline-design team for the technical deliverables review of this grant to advance:
|
Just a quick update. We are making a new standalone documentation site that should much better address outstanding questions. Will also get those other notes completed soon. Thanks for your patience! |
@SilentRhetoric Updated Readme: https://www.npmjs.com/package/@avmkit/siwa |
The SIWA website will be down while we are setting up the new docs site. |
@SilentRhetoric Here is the link to the security document. This is my first time working with Latex so I'm happy to update as needed. https://github.com/headline-design/avmkit/blob/main/packages/siwa/security.tex |
Added an attribution section to the Readme as well. In addition to SIWE, SIWA is also based on the excelent work done by Talisman on Sign In With Substrate and Phantom with Sign In With Solana. All three projects are referenced in the Readme now - https://github.com/headline-design/avmkit/tree/main/packages/siwa |
@SilentRhetoric Here is the AVM + EVM Template. I used the SSR AlgoStack template so we are calling it Algostack EVM. The main template repo is in avmkit/templates, but I also created a standalone repo for it for forking and it is deployed on Vercel as well. This integrates all the Algostack Algorand stack with Wagmi, Viem, Connectkit for full EVM integration. The prisma database accepts Algorand or EVM wallets as compatible user wallets. Here are the links: Deployed - https://algostack-evm.vercel.app. Standalone repo for forking - https://github.com/headline-design/algostack-evm. Templates repo - https://github.com/headline-design/avmkit/tree/main/templates/algostack-evm |
Thanks for the document, but i think this would need to be a technical document. It reads as an article. Claiming interoperability on a SIWA scheme with EVM / SIWE is a bold endeavour and personally I don't think you demonstrated that. We should be reading in that document about account abstraction, the use of ecdsa_verify for those who bring their secp256k1 keys and edDsa verify on the other side (EVM). You would have an easier delivery by just doing Algorand sign-in instructions of trying to be EVM friendly. This design prevents in doing exactly what you set out to deliver, for example:
You simply apply a hashing function on a public key to try to get another public key on a different curve! You can't even know if the result of the hash of a valid point? Even if you apply the secp256k1 curve equation to check if was so, you don't have a scalar for that "EVM"-compatible public key . In ECC you need to have a scalar and mult on the generation point. How would we even sign with that "EVM public key"? I would strongly discourage the current approach. It doesn't give AVM / EVM compatibility in any reasonable way |
Any luck with Pera & Defly? |
Working on it now |
Thanks for the feedback on this. The Algorand address conforms to EIP-55 with this approach. EIP-55 is a standard check that most EVM applications preform. We pass the native Algorand address for signing validation and the EIP-55 compatible address for EIP-55 compatibility. Are you suggesting we make that part optional though? Since the feature is for EIP-55 compatibility? Right now EIP-55 compatibilty is a required part of the message scheme. |
Extremely concerning. |
@headline-design it feels like you don't know what you're doing here. do you have a proper architecture you can share? I'm gravely concerned this will run in production. |
@ehanoc here is the Pera and Defly template - https://siwa-connect.vercel.app. Repo template - https://github.com/headline-design/avmkit/tree/main/templates/siwa-connect. Also I've got the full documentation site up now at https://docs.siwa.org. Individual integration pages for Pera - https://docs.siwa.org/integrations/pera-wallet and Defly - https://docs.siwa.org/integrations/defly-wallet. Also the docs repo is public so anyone can contribute directly as well at - https://github.com/headline-design/siwa-docs |
@johnalanwoods if you are referring to architectural diagram here is the basic diagram - https://docs.siwa.org/sign-in-with-algorand/quickstart-guide/sign-in-diagram. And here is the fullstack diagram - https://docs.siwa.org/general-information/siwa-overview/extended-siwa-diagram |
If the purpose of EIP-55 compatibility is confusing, I can remove it from the scheme. The derived address is not used for signing messages, only for EIP-55 compatibility. |
Ok, I'm going to go ahead and remove the EIP-55 check. will take some time to update the docs but should have all the templates and the main library updated tonight. |
Still have some work to do on docs. Added support now for Pera, Defly, Lute, and Kibisis wallets in SIWA Connect. Need to add new docs for that too and at least one more qc through all the templates to support the library updates. |
I mean, have you spoken with Pera, Defly and others to adopt this? Writing code mentioning this "integrations" as it was defined in the proposal doesn't mean this will be available in the wallets. |
This was part of the proposal and what the xGovs expected from the delivery point of view. The implementation doesn't seem correct to me though. If you want to move forward i would expect the AVM / EVM compatibility piece to be correctly delivered |
I have updated the docs to include full documentation and support for all 4 wallets. I have also integrated all four wallets with the SIWA Connect template. I also updated the main SIWA package to provide validation based on provider. For Pera and Kibisis we verify with verifyBytes. For Defly and Lute we verify with a seperate verifyTransaction utility. All four wallets have been tested and validated. Ideally all wallets would support verifyBytes as this is the optimal verifcation message. I made a Defly Connect request for adding verifyBytes for that as well. |
If you refer to the proposal and deliverables, the section on EVM-compatibility specifically refers to leveraging the Algorand blockchain for authentication. The Algostack EVM template fully demonstrates how seamlesly we can integrate Algorand authentication in an EVM application. The proposal makes no mention of signing EVM transaction or vice-versa, instead focusing specifically on standardizing how developers can add Algorand support to their apps. See here: "This is a critical feature for developers who build EVM-compatible applications and want to leverage the Algorand blockchain for authentication." Beyond this, I'm not entirely sure what you are looking for or expect to be delivered. If you refer to the abstract - "SIWA (Sign In With Algorand) is a decentralized authentication protocol that allows users to sign in to websites and applications using their Algorand wallet." Our methodolgy closely aligns with SIWE and EIP-4361. The entire EVM-integration feature here - "Feature 1: EVM Compatibility If there are specific things you are looking for beyond this, I may need your help in implementing them as you may be looking for something specifc that I cannot entirely discern. |
@ehanoc Also, here are the 4 individual wallet documentaion pages. Pera - https://docs.siwa.org/integrations/pera-wallet. Defly - https://docs.siwa.org/integrations/defly-wallet. Lute - https://docs.siwa.org/integrations/lute-wallet. Kibisis - https://docs.siwa.org/integrations/kibisis-wallet. Also I added an updated diagram to demonstrate how we handle verification for the different providers - https://docs.siwa.org/general-information/siwa-overview/sign-in-diagram. |
You don't generate valid ethereum addresses. They might look like having similar characters, but its not possible to find a valid public key on a different curve by hashing the previous one. For any EVM compatibility you would need to also be able to sign. |
I meant on the wallets themselves. |
No description provided.