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

[chain] Refactor Base into Transaction #1054

Closed
wants to merge 7 commits into from

Conversation

iFrostizz
Copy link
Contributor

Closes #685

Making the Base part of the Transaction so that their relationship is clearer.

Comment on lines -48 to -50
func (*Base) Size() int {
return BaseSize
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that Size(), even if defined, didn't seemed to be used. So I just deleted the whole file because all of the other methods have been inlined into Transaction.

cli/spam.go Outdated
Comment on lines 474 to 476
func (t *timeModifier) Base(b *chain.Transaction) {
b.Timestamp = t.Timestamp
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we really want the modifier to have access to the full Transaction ?

@iFrostizz iFrostizz force-pushed the refactor_base_into_transaction branch from b949d93 to 423c401 Compare June 19, 2024 16:14
@iFrostizz iFrostizz force-pushed the refactor_base_into_transaction branch from 92b1b77 to 68b525a Compare June 21, 2024 11:06
cli/spam.go Fixed Show fixed Hide fixed
cli/spam.go Outdated Show resolved Hide resolved
@iFrostizz iFrostizz self-assigned this Jun 21, 2024
cli/spam.go Fixed Show fixed Hide fixed
@iFrostizz iFrostizz force-pushed the refactor_base_into_transaction branch from 5166693 to 064b705 Compare June 25, 2024 15:07
@aaronbuchwald
Copy link
Collaborator

I don't think we want to use the modifier as it's used in this PR long term. Closing in favor of a different tx/action refactor later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[chain] refactor Base into Transaction
2 participants