Features/fix fulfilled #65
Conversation
Upgraded @btc-vision/btc-runtime to version 1.10.0 in package.json. Refactored test_helper.ts to use ExtendedAddress.toCSV instead of Address.toCSV for receiver address CSV exports, aligning with the updated runtime API.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a fulfilled provider queue mechanism to manage providers that have completed their liquidity obligations and need to be reset. The implementation adds a queuing system to defer provider resets, preventing excessive gas consumption from immediate resets during operations.
- Adds a new
fulfilledstate to providers with corresponding getter/setter methods and serialization support - Implements
FulfilledProviderQueueclass to batch process provider resets with configurable limits - Integrates fulfilled provider checks across reserve, listing, and cancellation operations to prevent actions on providers awaiting reset
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/operations/ReserveLiquidityOperation.ts | Adds fulfilled provider reset parameter and validation check to prevent reserving liquidity from fulfilled providers |
| src/operations/ListTokensForSaleOperation.ts | Integrates fulfilled provider reset call and adds validation to prevent listing from fulfilled providers |
| src/operations/CancelListingOperation.ts | Adds validation to prevent canceling listings for fulfilled providers |
| src/models/ProviderData.ts | Implements fulfilled state storage with getter/setter and serialization in the provider data model |
| src/models/Provider.ts | Adds fulfilled state management methods (isFulfilled, markFulfilled, clearFulfilled) and reformats a transfer call |
| src/managers/interfaces/IProviderManager.ts | Adds resetFulfilledProviders method to the interface |
| src/managers/interfaces/ILiquidityQueue.ts | Adds resetFulfilledProviders method to the interface |
| src/managers/TradeManager.ts | Adds check to skip fulfilled providers during trade processing |
| src/managers/PurgedProviderQueue.ts | Extends purged queue to conditionally add providers to fulfilled queue based on reset count threshold |
| src/managers/ProviderQueue.ts | Adds validation to ensure providers in the queue are not fulfilled |
| src/managers/ProviderManager.ts | Initializes fulfilled queues for normal and priority providers and implements reset logic |
| src/managers/LiquidityQueue.ts | Delegates fulfilled provider reset calls to the provider manager |
| src/managers/FulfilledProviderQueue.ts | New class implementing the fulfilled provider queue with batch reset functionality |
| src/contracts/NativeSwap.ts | Updates provider serialization to include fulfilled state and passes reset configuration parameters |
| src/constants/StoredPointers.ts | Adds storage pointers for normal and priority fulfilled queues |
| src/constants/Contract.ts | Defines maximum reset count constants for the fulfilled provider mechanism |
| src/tests/test_helper.ts | Updates to use ExtendedAddress.toCSV instead of Address.toCSV for test addresses |
| package.json | Updates @btc-vision/btc-runtime dependency from ^1.9.15 to ^1.10.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private ensureProviderIsNotFulfilled(): void { | ||
| if (this.provider.isFulfilled()) { | ||
| throw new Revert( | ||
| 'NATIVE_SWAP: Provider is in the reset queue and needs to be resets first. Try again in a few blocks.', |
There was a problem hiding this comment.
The word "resets" should be "reset" (singular verb form). The provider needs to be reset, not "resets".
| 'NATIVE_SWAP: Provider is in the reset queue and needs to be resets first. Try again in a few blocks.', | |
| 'NATIVE_SWAP: Provider is in the reset queue and needs to be reset first. Try again in a few blocks.', |
src/models/ProviderData.ts
Outdated
|
|
||
| /** | ||
| * @method fulfilled | ||
| * @description Gets if the provider is fulfilled and needs to be resets. |
There was a problem hiding this comment.
The word "resets" should be "reset" (singular verb form). The provider needs to be reset, not "resets".
| * @description Gets if the provider is fulfilled and needs to be resets. | |
| * @description Gets if the provider is fulfilled and needs to be reset. |
| private reservedTokens: u256 = u256.Zero; | ||
| private satoshisSpent: u64 = 0; | ||
| private readonly maximumProvidersPerReservation: u8; | ||
| private readonly numberOfFulfilledProviderToResets: u32; |
There was a problem hiding this comment.
The variable name numberOfFulfilledProviderToResets should be numberOfFulfilledProvidersToReset. The word "Provider" should be plural "Providers" (since it's a count of multiple providers), and "Resets" should be "Reset" (singular verb form).
| minimumAmountOutTokens: u256, | ||
| activationDelay: u8, | ||
| maximumProvidersPerReservation: u8, | ||
| numberOfFulfilledProviderToResets: u32, |
There was a problem hiding this comment.
The parameter name numberOfFulfilledProviderToResets should be numberOfFulfilledProvidersToReset. The word "Provider" should be plural "Providers" (since it's a count of multiple providers), and "Resets" should be "Reset" (singular verb form).
src/managers/PurgedProviderQueue.ts
Outdated
| protected readonly queue: StoredU32Array; | ||
| protected readonly enableIndexVerification: boolean; | ||
| protected readonly liquidityQueueReserve: ILiquidityQueueReserve; | ||
| private providerResetCount: u8; |
There was a problem hiding this comment.
The providerResetCount should be reset to 0 after processing is complete (e.g., at the end of the get method or in a cleanup method). Without resetting this counter, it will keep accumulating across multiple calls, which may not be the intended behavior for tracking resets per operation/batch.
| private ensureProviderIsNotFulfilled(): void { | ||
| if (this.provider.isFulfilled()) { | ||
| throw new Revert( | ||
| 'NATIVE_SWAP: You cannot cancel this listing at the moment. Provider is in the reset queue and needs to be resets first. Try again in a few blocks.', |
There was a problem hiding this comment.
The word "resets" should be "reset" (singular verb form). The provider needs to be reset, not "resets".
| 'NATIVE_SWAP: You cannot cancel this listing at the moment. Provider is in the reset queue and needs to be resets first. Try again in a few blocks.', | |
| 'NATIVE_SWAP: You cannot cancel this listing at the moment. Provider is in the reset queue and needs to be reset first. Try again in a few blocks.', |
src/constants/Contract.ts
Outdated
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDER_TO_RESETS_BEFORE_QUEUING: u8 = 40; | ||
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDER_TO_RESETS: u32 = 80; |
There was a problem hiding this comment.
The constant name MAXIMUM_NUMBER_OF_PURGED_PROVIDER_TO_RESETS should be MAXIMUM_NUMBER_OF_PURGED_PROVIDERS_TO_RESET. The word "PROVIDER" should be plural "PROVIDERS" (since it's a count of multiple providers), and "RESETS" should be "RESET" (singular verb form).
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDER_TO_RESETS_BEFORE_QUEUING: u8 = 40; | |
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDER_TO_RESETS: u32 = 80; | |
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDERS_TO_RESET_BEFORE_QUEUING: u8 = 40; | |
| export const MAXIMUM_NUMBER_OF_PURGED_PROVIDERS_TO_RESET: u32 = 80; |
| const countToResets: u32 = min(count, this.queue.getLength()); | ||
|
|
||
| for (let index: u32 = 0; index < countToResets; index++) { |
There was a problem hiding this comment.
The variable name countToResets should be countToReset. The word "Resets" should be "Reset" (singular verb form) as it represents the count of items to reset, not multiple reset operations.
| const countToResets: u32 = min(count, this.queue.getLength()); | |
| for (let index: u32 = 0; index < countToResets; index++) { | |
| const countToReset: u32 = min(count, this.queue.getLength()); | |
| for (let index: u32 = 0; index < countToReset; index++) { |
src/managers/PurgedProviderQueue.ts
Outdated
| result = provider; | ||
| } else if (!provider.hasReservedAmount()) { | ||
| this.resetProvider(provider, associatedQueue); | ||
| if (this.providerResetCount > this.maximumResetsBeforeQueuing) { |
There was a problem hiding this comment.
The comparison logic appears incorrect. The condition this.providerResetCount > this.maximumResetsBeforeQueuing means that providers will be added to the fulfilled queue only after exceeding the maximum. However, based on the variable name maximumResetsBeforeQueuing, it seems the intent is to start queuing once the maximum is reached. The comparison should likely be >= instead of > to properly enforce the maximum limit.
| if (this.providerResetCount > this.maximumResetsBeforeQueuing) { | |
| if (this.providerResetCount >= this.maximumResetsBeforeQueuing) { |
Updated provider and purged provider queue tests to use new FulfilledProviderQueue and to pass additional arguments as required by recent interface changes. Updated instantiation and usage patterns in test files for queue management. Changed @btc-vision/btc-runtime dependency to a local file reference in package.json. Improved error handling in as-pect.config.js. No production logic changes, only test and configuration updates.
Updated purge and restore logic to pass the current quote through ReservationManager and ProviderManager, ensuring accurate minimum reservation checks. Adjusted several constants in Contract.ts to new values for provider and reservation limits. Refactored LiquidityQueue's quote method to support recalculation and moved quote calculation logic to a private method. Updated interfaces and usages accordingly for consistency.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private ensureProviderIsNotFulfilled(): void { | ||
| if (this.provider.toReset()) { | ||
| throw new Revert( | ||
| 'NATIVE_SWAP: Provider is in the reset queue and needs to be resets first. Try again in a few blocks.', |
There was a problem hiding this comment.
The error message refers to "resets" instead of "reset". This should be "reset" for grammatical correctness.
Suggested change:
'NATIVE_SWAP: Provider is in the reset queue and needs to be reset first. Try again in a few blocks.'
|
|
||
| /** | ||
| * @method toReset | ||
| * @description Gets if the provider is fulfilled and needs to be resets. |
There was a problem hiding this comment.
The documentation refers to "resets" instead of "reset". This should be "reset" for grammatical correctness.
|
|
||
| /** | ||
| * @method toReset | ||
| * @description Sets if the provider is fulfilled and needs to be resets. |
There was a problem hiding this comment.
The documentation refers to "resets" instead of "reset". This should be "reset" for grammatical correctness.
|
|
||
| /** | ||
| * @method toReset | ||
| * @description Gets if the provider needs to be resets. |
There was a problem hiding this comment.
The documentation refers to "resets" instead of "reset". This should be "reset" for grammatical correctness.
| /** | ||
| * @method toReset | ||
| * @description Gets if the provider needs to be resets. | ||
| * @returns {boolean} - true if the provider needs to be resets; false if not. |
There was a problem hiding this comment.
The documentation refers to "resets" instead of "reset". This should be "reset" for grammatical correctness.
as-pect.config.js
Outdated
| //entries: ['src/__tests__/**/*.spec.ts'], | ||
| entries: ['src/__tests__/**/fulfilledProviderQueue_tests.spec.ts'], |
There was a problem hiding this comment.
The test configuration has been changed to only run fulfilledProviderQueue_tests.spec.ts. This should be reverted to run all tests before merging.
entries: ['src/__tests__/**/*.spec.ts'],
//entries: ['src/__tests__/**/fulfilledProviderQueue_tests.spec.ts'],| //entries: ['src/__tests__/**/*.spec.ts'], | |
| entries: ['src/__tests__/**/fulfilledProviderQueue_tests.spec.ts'], | |
| entries: ['src/__tests__/**/*.spec.ts'], | |
| //entries: ['src/__tests__/**/fulfilledProviderQueue_tests.spec.ts'], |
| queue.add(provider); | ||
| const index = purgedQueue.add(provider); | ||
|
|
||
| const index = purgedQueue.add(provider); |
There was a problem hiding this comment.
Unused variable index.
| const index = purgedQueue.add(provider); | |
| purgedQueue.add(provider); |
| } | ||
|
|
||
| const liquiditySum: u256 = liquidity.reduce( | ||
| (acc: u256, current: u256) => (acc = u256.add(acc, current)), |
There was a problem hiding this comment.
The value assigned to acc here is unused.
| (acc: u256, current: u256) => (acc = u256.add(acc, current)), | |
| (acc: u256, current: u256) => u256.add(acc, current), |
Upgraded @btc-vision/as-bignum and @btc-vision/btc-runtime dependencies in package.json. Modified several contract constants: reduced RESERVATION_EXPIRE_AFTER_IN_BLOCKS from 8 to 5, set TIMEOUT_AFTER_EXPIRATION_BLOCKS to 0, and increased MAX_PRICE_IMPACT_BPS and MAX_CUMULATIVE_IMPACT_BPS in Contract.ts.
…n/native-swap into features/fix_fulfilled
In LiquidityQueue, inlined and updated the quote calculation logic to improve clarity and error handling. Added REMOVAL_QUEUE_POINTER and REMOVAL_QUEUE_PURGED_RESERVATION to StoredPointers. Removed unnecessary debug logs from ProviderQueue. Enhanced error message in ReserveLiquidityOperation for insufficient fees.
…n/native-swap into features/fix_fulfilled
Removal queue related constants in StoredPointers.ts have been commented out, indicating they are no longer in use. Additionally, a large block of legacy slashing logic in ListTokensForSaleOperation.ts has been commented out, likely in favor of a newer implementation.
Removed unused penalty accrual and slashing constants, refactored slashing activation to use net amounts and simplified price impact checks, improved ceiling division for virtual reserve calculations, and clarified token-to-satoshis conversion. Also updated SwapOperation to distinguish between pre-fee and post-fee token amounts for pool and reserve tracking.
Improves accuracy in token reservation by capping reserved tokens to prevent rounding errors, updates satoshis-to-tokens and tokens-to-satoshis conversion to handle rounding up, and refactors related methods for clarity. Also simplifies Provider's liquidity check and adds explanatory comments for liquidity updates and tax deductions.
Refactor liquidity pool logic and improve slashing checks
…ulation When a provider is activated during their first swap, the second 50% of their listing is calculated from their remaining liquidity after tokens are sold, not their original listing amount. This causes the virtual pool to receive less tokens than expected, breaking price discovery.
Bug: Provider Activation Uses Post-Sale Liquidity for Second 50% Calc…
…n/native-swap into features/fix_fulfilled
Introduces stable pool type and amplification coefficient for improved pricing and slippage control in liquidity pools. Updates pool creation, management, and quoting logic to support both standard and stable pools, including new constants, storage pointers, and validation checks.
Introduces OP20StableQuery for querying stable token peg rates, authority, and update times. Extends liquidity pool creation and management to support peg rate scaling, staleness threshold, and StableSwap math centered around the peg. Updates interfaces and pool creation logic to require IOP20Stable compliance for stable pools.
Simplifies the import statement and updates the applyStableSwapSell method to use the original invariant D when computing newB, ensuring B is solved to maintain D after the swap.
Updated stableQuoteRaw to correct formula for numerator and denominator, using 4A instead of A*B and adjusting division terms. Simplified updateVirtualPoolStable to maintain invariant D through swaps, removed unnecessary recomputation of D, and clarified logic for applying token sells and buys.
Introduces MIN_PEG_RATE and MAX_PEG_RATE constants for defensive validation of peg rates in Contract.ts. Updates LiquidityQueue.ts to use these bounds, refines StableSwap pool logic, improves error messages, and adds additional sanity checks to prevent overflow and underflow in pool computations.
Introduced a check in OP20StableQuery to prevent future peg timestamps from malicious tokens. Added a zero-reserve check in LiquidityQueue's computeStableY to avoid invalid computations.
Add stable pool support OP-20S (Stable extension)
…n/native-swap into features/fix_fulfilled
This PR introduces a fulfilled provider queue mechanism to manage providers that have completed their liquidity obligations and need to be reset. The implementation adds a queuing system to defer provider resets, preventing excessive gas consumption from immediate resets during operations.