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

feat: add versioned parameters to payment module #281

Merged

Conversation

forcodedancing
Copy link
Contributor

Description

This pr will add versioned parameter to payment module.
Currently, ReserveTime is a multiple version parameter.

Rationale

When unlock creation fee, we need to know the exact amount locked.

Example

NA

Changes

Notable changes:

  • payment parameter

@forcodedancing forcodedancing requested a review from owen-reorg June 8, 2023 09:33
@owen-reorg
Copy link
Contributor

Better to have e2e cases to cover the usage.

  1. create an object, increase the reserve time, seal the object. Previously this should return error, but with the versioned params it works
  2. create a bucket with non-zero read quota, change the validator tax rate, delete the bucket. The rate of the validator tax address should be zero. Before this change, it will be messed up.

@owen-reorg
Copy link
Contributor

There are still chances that delete an object may fail after this fix.
E.g. User A has 5KB objects with reserve time of 100 seconds, then the reserve times was changed to 200 seconds. Even if User A delete 1KB, the total balance is not enough for 4KB * 200s, so the delete action will fail, unless User A deposit more BNB to his stream account.

So the delete object function in Endblock has to handle this scenario.

@forcodedancing
Copy link
Contributor Author

forcodedancing commented Jun 10, 2023

Better to have e2e cases to cover the usage.

  1. create an object, increase the reserve time, seal the object. Previously this should return error, but with the versioned params it works
  2. create a bucket with non-zero read quota, change the validator tax rate, delete the bucket. The rate of the validator tax address should be zero. Before this change, it will be messed up.

Added new E2E testcases into payment e2e test.

@forcodedancing forcodedancing force-pushed the feat/payment_multi_version_params branch from 15ac3df to 6c698f4 Compare June 10, 2023 12:19
@forcodedancing
Copy link
Contributor Author

There are still chances that delete an object may fail after this fix. E.g. User A has 5KB objects with reserve time of 100 seconds, then the reserve times was changed to 200 seconds. Even if User A delete 1KB, the total balance is not enough for 4KB * 200s, so the delete action will fail, unless User A deposit more BNB to his stream account.

So the delete object function in Endblock has to handle this scenario.

As discussed, this should be ok. Added new testcase to cover this.

@forcodedancing forcodedancing force-pushed the feat/payment_multi_version_params branch from d3fafd0 to 0a372da Compare June 12, 2023 08:19
@forcodedancing forcodedancing force-pushed the feat/payment_multi_version_params branch from ff12405 to 47faece Compare June 12, 2023 08:41
@forcodedancing forcodedancing requested a review from unclezoro June 13, 2023 06:03
@unclezoro unclezoro merged commit 320ee1f into bnb-chain:develop Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants