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

Cleanup internals so that we don't need Signer #153

Open
ChaoticTempest opened this issue Jun 20, 2022 · 2 comments
Open

Cleanup internals so that we don't need Signer #153

ChaoticTempest opened this issue Jun 20, 2022 · 2 comments

Comments

@ChaoticTempest
Copy link
Member

ChaoticTempest commented Jun 20, 2022

The Signer type isn't necessarily needed to be apart of the internals. This was originally there to first get everything to work as intended, but having a dependency on this type from near-primitives without any tests can break things when we go to update dependencies. So either add tests or clean up the internals. I'd vote for cleaning up internals, since I can see this snowballing where we'd have to eventually clean it up and adding further technical debt down the line with it being used more and more.

Note that cleaning up the Signer type will require us grabbing the SignedTransaction::from_actions function and all related code dependencies which could be a lot.

@ghost ghost mentioned this issue Oct 17, 2023
@frol
Copy link
Collaborator

frol commented Oct 29, 2023

@ChaoticTempest Does #329 address your point here? To be honest, I don't see any problem with using Signer in the first place.

@ChaoticTempest
Copy link
Member Author

ChaoticTempest commented Oct 30, 2023

@frol as far as I remember, yes it should accomplish this ticket. The main reason why I think I wanted to move away from it was because SignedTransaction::from_actions was/is apart a test_utils file in nearcore and seemed not so great to be using it directly just in case it got removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: NEW❗
Development

No branches or pull requests

2 participants