Release/0.11.6 #12
Release/0.11.6 #12
Conversation
* update sdk * sdk-3.11.3
feature/SP-6435, update SDK
fix: Offer restrictions schema
* feat(replenish-deposit): Add replenish workflow cmd * feat(replenish-deposit): add option --yes * fix(replenish-deposit): add parser for --minutes option * fix(replenish-deposit): change minutes logic --------- Co-authored-by: Petr Evstifeev <7293601+Villain88@users.noreply.github.com>
* feat(offer version): add offer version command * fix(offer version): change option to requiredOption * fix(offer version): update code after tests * fix(update version): ai review changes * chore(update version): update log messages * fix(offer versions): review changes * fix(offer-version): argument parsing in cmd
…latest fix(create-order): Use fromBlock toBlock
WalkthroughThis update introduces new CLI commands for managing offer versions and workflow deposit replenishments, adds supporting service modules for reading and validating offer version info, and updates dependency versions. Schema definitions are refined for offer restrictions, and a utility for parsing numbers is added. Existing logic remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant BlockchainService
participant FileService
User->>CLI: Run 'offer version add' with params
CLI->>FileService: Read and validate version info file
FileService-->>CLI: Return validated version info
CLI->>BlockchainService: Add new offer version with info
BlockchainService-->>CLI: Confirm addition
CLI-->>User: Print success or error message
sequenceDiagram
participant User
participant CLI
participant BlockchainService
User->>CLI: Run 'workflows replenish-deposit' with params
CLI->>BlockchainService: Fetch order status
BlockchainService-->>CLI: Return order status
CLI->>User: Prompt for confirmation (unless silent)
User-->>CLI: Confirm (if prompted)
CLI->>BlockchainService: Replenish deposit
BlockchainService-->>CLI: Confirm replenishment
CLI-->>User: Print result
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/utils.ts (1)
203-207: Be stricter when validating CLI numeric arguments
Number('')returns0andNumber('Infinity')returnsInfinity, both of which silently pass the current check but are very unlikely to be valid values for things like a version or timeout. Consider rejecting empty / whitespace‐only strings and anything that is not a finite decimal number.export const parseNumber = (val: string): number => { - const parsed = Number(val); - if (Number.isNaN(parsed)) throw new InvalidArgumentError('Not a number.'); - return parsed; + const trimmed = val.trim(); + if (!/^-?\d+(\.\d+)?$/.test(trimmed)) { + throw new InvalidArgumentError('Expected a decimal number.'); + } + + const parsed = Number(trimmed); + if (!Number.isFinite(parsed)) { + throw new InvalidArgumentError('Number is not finite.'); + } + return parsed; };src/services/readOfferVersionInfo.ts (2)
3-3: Use a relative import without the unnecessary directory hopThe file lives in the same folder as
readResourceFile.ts, so../services/...walks one level up and then immediately back down. Using a direct relative path is clearer and avoids breakage if the folder structure ever changes.-import { HashValidator } from '../services/readResourceFile'; +import { HashValidator } from './readResourceFile';
16-23: Propagate the validator’s type to avoid an extra castIf
readJsonFileis generic, pass the expected type so the result is strongly typed without an explicit cast:const offerOption = await readJsonFile<OfferVersionInfo>({ path: params.path, validator: OfferVersionInfoSchema, });This catches schema ↔ type mismatches at compile time.
src/services/createOrder.ts (1)
12-22: Typo in variable name and minor ordering concern
workflowCreationBLockhas a capital “L” in the middle – reads oddly and may trip linters.- Capturing the last block before
Orders.createOrdermeans the order itself will be created in a later block, but you start scanning from the earlier height anyway. That’s fine, just making sure it was intentional.- const workflowCreationBLock = await BlockchainConnector.getInstance().getLastBlockInfo(); + const workflowCreationBlock = await BlockchainConnector.getInstance().getLastBlockInfo(); ... - workflowCreationBLock.index, + workflowCreationBlock.index,src/commands/offerAddVersion.ts (2)
18-21: ReturnedactionAddressis computed but then discarded
initBlockchainConnectorresolves with the address derived fromactionAccountKey, yet the value is neither used nor verified.
IfaddValueOfferVersionneeds the address, pass it; otherwise call the connector for its side-effects once and rename to make this explicit.
23-28: Ambiguous field nameactionin service callYou pass the private key via the property
action, whereas the rest of the codebase consistently usesactionAccountKey.
Keeping the same naming avoids confusion and unintended structural typing mistakes.src/index.ts (2)
479-512: Option parsing duplicates logic & bypassesparseNumber
--minutesuses a customargParser(parseInt)despite having the newparseNumberhelper. This duplicates logic and bypasses central validation (e.g., negative values).
ReuseparseNumberhere for consistency.- ).argParser((val) => parseInt(val) || 0), + ).argParser(parseNumber),
1460-1478: Duplicate root commands:offervsoffersThe CLI now features both
offers …and singleoffer version …command trees. This confuses users and breaks tab-completion. Prefer nesting version commands under the existingoffersroot (e.g.,offers version add).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.json(2 hunks)src/commands/offerAddVersion.ts(1 hunks)src/commands/offerDisableVersion.ts(1 hunks)src/commands/workflowsReplenishDeposit.ts(1 hunks)src/index.ts(5 hunks)src/services/createOrder.ts(2 hunks)src/services/readOfferVersionInfo.ts(1 hunks)src/services/readValueOfferInfo.ts(1 hunks)src/utils.ts(2 hunks)
🔇 Additional comments (4)
src/utils.ts (1)
3-3: Ensure Commander version ≥ 8.0 is locked in
InvalidArgumentErrorships with Commander 8+.package.jsoncurrently has"commander": "^8.3.0", which is fine, but please keep an eye on this if you ever downgrade or pin a lower major version.package.json (1)
3-3: Version bumps look saneBumping
@super-protocol/ctlto 0.11.6 aligns the CLI with the new commands. No issues spotted here.src/services/readValueOfferInfo.ts (1)
19-26: Down-stream code may still expectoffers: string[]
offersis now an array of{ id, version }. Any existing logic that didoffers.includes(...), treated items as strings, or relied on the old schema will break at runtime.Please grep the project (including tests) for:
rg --context 2 '\.offers'and update usages accordingly.
src/index.ts (1)
502-509: Potential undefinedminutes/sppipass-through
WorkflowReplenishDepositParams.minutesis declarednumber, but can beundefinedwhen only--sppiis supplied, violating the type contract and risking runtimeNaNchecks inside the service.
Mark the fields optional or ensure defaults before passing.
| export type OffersDisableVersionParams = { | ||
| blockchainConfig: BlockchainConfig; | ||
| actionAccountKey: string; | ||
| id: string; | ||
| ver: number; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Input contract lacks basic sanity checks
id might be an empty string and ver could be <1; this will go straight to the chain and revert.
Add lightweight guards before the blockchain call.
🤖 Prompt for AI Agents
In src/commands/offerDisableVersion.ts between lines 5 and 10, the
OffersDisableVersionParams type lacks validation for the id and ver fields. Add
checks before the blockchain call to ensure id is not an empty string and ver is
at least 1. If these conditions are not met, throw an error or return early to
prevent sending invalid data to the blockchain and causing a revert.
| } catch (error: any) { | ||
| Printer.print('Remove offer version failed with error: ' + error?.message); | ||
| } |
There was a problem hiding this comment.
Swallowed error obstructs CLI exit status
The catch block only logs the error. The CLI that invokes this command will interpret a resolved promise as success, masking failures (e.g., CI/CD scripts will think the version was removed).
After logging, re-throw the error or process.exit(1) so the caller can react.
- } catch (error: any) {
- Printer.print('Remove offer version failed with error: ' + error?.message);
+ } catch (error: unknown) {
+ Printer.print(`Remove offer version failed: ${(error as Error).message}`);
+ throw error; // let the upper layer decide how to handle/exit
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error: any) { | |
| Printer.print('Remove offer version failed with error: ' + error?.message); | |
| } | |
| } catch (error: unknown) { | |
| Printer.print(`Remove offer version failed: ${(error as Error).message}`); | |
| throw error; // let the upper layer decide how to handle/exit | |
| } |
🤖 Prompt for AI Agents
In src/commands/offerDisableVersion.ts around lines 22 to 24, the catch block
logs the error but does not propagate it, causing the CLI to treat failures as
successes. To fix this, after logging the error message, either re-throw the
caught error or call process.exit(1) to ensure the CLI exits with a failure
status and the caller can handle the error properly.
| export default async (params: OfferAddVersionParams): Promise<void> => { | ||
| try { | ||
| Printer.print('Connecting to the blockchain'); | ||
| await initBlockchainConnector({ | ||
| blockchainConfig: params.blockchainConfig, | ||
| actionAccountKey: params.actionAccountKey, | ||
| }); | ||
| const versionInfo = await readOfferVersionInfo({ path: params.path }); | ||
| const newVersion = await addValueOfferVersion({ | ||
| action: params.actionAccountKey, | ||
| version: params.ver, | ||
| versionInfo, | ||
| offerId: params.id, | ||
| }); | ||
| Printer.print(`Version ${newVersion} added successfully`); | ||
| } catch (error: any) { | ||
| Printer.print('Version add failed with error: ' + error?.message); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing propagation of failures
Same as in offerDisableVersion.ts: the error is logged but not re-thrown, so upstream code can’t tell the command failed.
🤖 Prompt for AI Agents
In src/commands/offerAddVersion.ts around lines 15 to 33, the catch block logs
the error but does not re-throw it, preventing upstream code from detecting the
failure. Modify the catch block to re-throw the caught error after logging it,
ensuring that the failure propagates properly to calling functions.
| } catch (error: any) { | ||
| Printer.print(`Order ${params.orderId} was not replenished: ${error?.message}`); | ||
| } |
There was a problem hiding this comment.
Error object typed as any and swallowed
Re-throw after logging (see comments in other files) so the CLI command reflects failure status and exit code.
🤖 Prompt for AI Agents
In src/commands/workflowsReplenishDeposit.ts around lines 70 to 72, the catch
block types the error as any and only logs it without re-throwing. Modify the
catch block to re-throw the caught error after logging the message so that the
CLI command properly reflects the failure status and exit code.
| const confirmed = | ||
| params.yes || | ||
| (await isActionConfirmed( | ||
| `Deposit will be replenished by ${weiToEther(tokenAmount)} SPPI. Order time is extended by ${minutes} minutes.`, | ||
| )); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Dual-input ambiguity
If the user provides both --minutes and --sppi, the first if (params.minutes) branch silently wins. Make this mutually exclusive and throw if both are set.
🤖 Prompt for AI Agents
In src/commands/workflowsReplenishDeposit.ts around lines 60 to 64, the code
allows both --minutes and --sppi parameters to be set simultaneously, but only
processes the --minutes branch silently. To fix this, add a check before this
logic to detect if both params.minutes and params.sppi are set, and if so, throw
an error indicating that these options are mutually exclusive. This prevents
ambiguity by enforcing that only one of these inputs can be provided at a time.
| if (params.minutes) { | ||
| tokenAmount = (BigInt(amountPerHour) * BigInt(params.minutes)) / 60n; | ||
| minutes = params.minutes; | ||
| } else if (params.sppi) { | ||
| tokenAmount = etherToWei(params.sppi).toBigInt(); | ||
| minutes = Number((tokenAmount * 60n) / BigInt(amountPerHour)); | ||
| } else { | ||
| throw new Error('To complete command please define one of arguments --sppi or --minutes'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Calculation loses precision for non-divisible minute values
tokenAmount = amountPerHour * minutes / 60 truncates towards zero (BigInt division), potentially under-funding the order.
Example: amountPerHour = 100, minutes = 59 → tokenAmount = 98 instead of 98.333….
Consider ceiling division or informing the user about the rounding strategy.
🤖 Prompt for AI Agents
In src/commands/workflowsReplenishDeposit.ts around lines 50 to 58, the
calculation of tokenAmount uses integer division with BigInt, which truncates
and loses precision for non-divisible minute values. To fix this, implement a
ceiling division approach to round up the result instead of truncating, ensuring
the tokenAmount does not under-fund the order. Adjust the formula to calculate
tokenAmount as (amountPerHour * minutes + 59) / 60 or an equivalent method to
achieve ceiling division with BigInt.
| offerVersionCommand | ||
| .command('add') | ||
| .description('Add new version of offer') | ||
| .requiredOption('--offer <id>', 'Offer id') | ||
| .requiredOption('--path <path>', 'Path to offer version info file') | ||
| .option('--ver <version>', 'Version number to add', parseNumber) | ||
| .action(async (options: { offer: string; path: string; ver: number; config: string }) => { | ||
| const configLoader = new ConfigLoader(options.config); | ||
| const blockchain = configLoader.loadSection('blockchain'); | ||
| const blockchainConfig = { | ||
| contractAddress: blockchain.smartContractAddress, | ||
| blockchainUrl: blockchain.rpcUrl, | ||
| }; | ||
| await offerAddVersion({ | ||
| actionAccountKey: blockchain.accountPrivateKey, | ||
| blockchainConfig, | ||
| id: options.offer, | ||
| ver: options.ver, | ||
| path: options.path, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ver is optional in CLI but required in service type
When --ver is omitted, options.ver is undefined, yet OfferAddVersionParams.ver is non-optional. TypeScript will compile due to any, but at runtime the service receives undefined and may fail on-chain.
Either:
- Make the CLI option
.requiredOption, or - Change the param to
ver?: numberand let the service auto-increment.
🤖 Prompt for AI Agents
In src/index.ts around lines 1438 to 1458, the CLI option --ver is currently
optional but the service expects a non-optional version number, causing runtime
issues if omitted. To fix this, either change the CLI option to
.requiredOption('--ver <version>', ...) to enforce providing a version, or
update the service parameter type to make ver optional (ver?: number) and
implement logic to auto-increment the version when it is not provided. Choose
one approach and update the code accordingly to ensure ver is always valid when
passed to the service.
Summary by CodeRabbit
@super-protocol/ctland@super-protocol/sdk-js.