Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Initialize user accounts for block generators / Fees in sidechains #239

Merged
merged 20 commits into from
Feb 16, 2023

Conversation

mjerkov
Copy link
Contributor

@mjerkov mjerkov commented Jan 24, 2023

This PR aims at solving the following issues:

Initialize user accounts for block generators

Problem statement

It is possible for sidechains that, while assigning rewards to block generator (either block rewards or unburned transaction fees), a user account for the corresponding token (reward/ fee token) does not exist for the generator, leading to this generator being unable to add blocks.

Proposed solution

  1. In LIP 0042 and LIP 0071: In afterTransactionsExecute, only mint tokens if the corresponding block generator account is initialized (either for TOKEN_ID_REWARD or TOKEN_ID_DYNAMIC_BLOCK_REWARD). If not, emit the event stating that no account is initialized.

  2. In LIP 0048: In afterCommandExecute, distinguish the following cases:

Case generatorAddress initialized generatorAddress not initialized
ADDRESS_FEE_POOL
exists and is initialized
same as current logic for
ADDRESS_FEE_POOL exists
all fees transferred to ADDRESS_FEE_POOL
ADDRESS_FEE_POOL
does not exist or is not initialized
same as current logic for
ADDRESS_FEE_POOL does not exist
all fees burned

Fees in sidechains

Problem statement

Solve the problems related to using LSK as default token for fees in a sidechain, e.g:
Is it actually possible to use LSK as default token for fees in a sidechain? To bring LSK tokens to the sidechain to begin with you have in general to pay the transaction fee of the CCU.

As discussed, the following issues were detected:

  1. The verify hook in the block processing of the Fee module would fail if MIN_FEE_PER_BYTE > 0, TOKEN_ID_FEE is the LSK token ID and the trs sender does not yet have any LSK on the sidechain.

  2. The account initialization in the CCU execution for the relayer may fail if the relayer account is not initialized and the account does not hold enough LSK (see if ccu.params.inboxUpdate is not empty check in LIP53).

Proposed solution

  1. In Lip 0048, in the block processing verify hook: Allow for MIN_FEE_PER_BYTE = 0 for a MAX_BLOCK_HEIGHT_ZERO_FEE_PER_BYTE number of blocks after the genesis block.

  2. This is not a protocol-related issue and would only be documented in the guidances for sidechains as something to keep in mind when creating the genesis block (e.g., "Set the LSK balance to 0 for all initial validators, so they can act as initial relayers").

@mjerkov mjerkov self-assigned this Jan 24, 2023
@mjerkov mjerkov linked an issue Jan 24, 2023 that may be closed by this pull request
Copy link
Contributor

@vjaiman vjaiman left a comment

Choose a reason for hiding this comment

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

So far looks good. But I suppose we are waiting for the conclusion on the GitHub discussion.
Also, in my opinion, it would be nice to do the cosmetic changes in a separate PR (if they are a lot) to avoid the large diff.

@mjerkov mjerkov requested a review from vjaiman February 2, 2023 07:46
@mjerkov
Copy link
Contributor Author

mjerkov commented Feb 2, 2023

@vjaiman @gkoumout As I already informed Greg, I didn't know how to make the new Token module method initializeUserAccountForBlockGenerator dedicated/callable only in the case of free account initialization for block generators. If you have any suggestions how to do it or is necessary, that would be great.

proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
@mjerkov mjerkov changed the title Initialize user accounts for block generators (fees/rewards) Initialize user accounts for block generators / Fees in sidechains Feb 8, 2023
@mjerkov mjerkov linked an issue Feb 8, 2023 that may be closed by this pull request
Copy link
Contributor

@vjaiman vjaiman left a comment

Choose a reason for hiding this comment

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

Good job Miroslav. I have left few changes.

proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Show resolved Hide resolved
proposals/lip-0071.md Outdated Show resolved Hide resolved
proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Good job Miroslav! I left some comments here and there that could be improved, but I think we are in a good stage.

proposals/lip-0071.md Outdated Show resolved Hide resolved
proposals/lip-0071.md Outdated Show resolved Hide resolved
proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0048.md Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@mjerkov mjerkov left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestions. I will update the PR with a new commit.

proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0042.md Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0071.md Show resolved Hide resolved
proposals/lip-0071.md Outdated Show resolved Hide resolved
@mjerkov mjerkov requested a review from vjaiman February 10, 2023 11:55
Copy link
Contributor

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Well done Miroslav! Just few minor comments. The only thing remaining to finalize is the initialization of ADDRESS_FEE_POOL account.

proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0071.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0042.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
Copy link
Contributor

@vjaiman vjaiman left a comment

Choose a reason for hiding this comment

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

Good job, Miroslav!

Copy link
Contributor

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Found also one broken link, need to fix it before merging.

proposals/lip-0071.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Well done Miroslav!

@mjerkov mjerkov requested a review from janhack February 14, 2023 11:41
proposals/lip-0048.md Outdated Show resolved Hide resolved
proposals/lip-0048.md Outdated Show resolved Hide resolved
@janhack janhack merged commit 625da22 into main Feb 16, 2023
@janhack janhack deleted the initialize-accounts-for-block-generators branch February 16, 2023 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initialize fee/reward token accounts for block generators Use LSK as a fee token in a sidechain
5 participants