Skip to content

Remove Custom transactions and make the stake transactions top level#149

Merged
majecty merged 3 commits intoCodeChain-io:masterfrom
sgkim126:stake
Apr 1, 2020
Merged

Remove Custom transactions and make the stake transactions top level#149
majecty merged 3 commits intoCodeChain-io:masterfrom
sgkim126:stake

Conversation

@sgkim126
Copy link
Contributor

@sgkim126 sgkim126 commented Feb 9, 2020

It closes #38

@majecty majecty changed the title [wip] Add stake transactions [WIP] Add stake transactions Feb 12, 2020
@kseo kseo added the do-not-merge Do not merge this PR label Mar 9, 2020
@sgkim126 sgkim126 changed the title [WIP] Add stake transactions Remove Custom transactions and make the stake transactions top level Mar 28, 2020
@sgkim126
Copy link
Contributor Author

sgkim126 commented Mar 28, 2020

It depends on #291

@sgkim126 sgkim126 force-pushed the stake branch 5 times, most recently from 1816016 to a823bda Compare March 30, 2020 09:08
@sgkim126 sgkim126 removed the do-not-merge Do not merge this PR label Mar 30, 2020
@sgkim126 sgkim126 requested a review from majecty March 30, 2020 15:33
// FIXME
0
}
_ => self.min_custom_transaction_cost,
Copy link

Choose a reason for hiding this comment

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

How about creating match cases for each Actions Instead of using _ pattern?
If we use _ pattern in our code, it is hard to find the match expression when we add a new enum variant in the Action.

However, if this code will be removed and all transactions will be moved to the Module level, it's good to use _ pattern here for temporary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#292 will remove the minimum cost. I'll rebase it on #292.

// FIXME
0
}
_ => params.min_custom_transaction_cost(),
Copy link

Choose a reason for hiding this comment

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

Will you change the name min_custom_transaction_cost later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto.

@majecty majecty merged commit 2b3636f into CodeChain-io:master Apr 1, 2020
@sgkim126 sgkim126 deleted the stake branch April 2, 2020 02:08
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.

Change stake transactions to first-class transactions instead of custom transactions

3 participants