Skip to content

Support amount alias for manageBuyOffer#973

Closed
chrisx9z wants to merge 1 commit into
stellar:masterfrom
chrisx9z:fix/manage-buy-offer-amount-alias
Closed

Support amount alias for manageBuyOffer#973
chrisx9z wants to merge 1 commit into
stellar:masterfrom
chrisx9z:fix/manage-buy-offer-amount-alias

Conversation

@chrisx9z

@chrisx9z chrisx9z commented Jun 1, 2026

Copy link
Copy Markdown

Summary

  • add non-breaking amount input support for Operation.manageBuyOffer
  • keep the existing buyAmount field for compatibility
  • expose amount alongside buyAmount when parsing manage buy offers from XDR
  • cover the alias in runtime and TypeScript declaration tests

Fixes #200.

Verification

  • node --check src/operations/manage_buy_offer.js
  • node --check src/operation.js
  • node --check test/unit/operations/classic_ops_test.js
  • corepack yarn mocha test/unit/operations/classic_ops_test.js --grep manageBuyOffer --require @babel/register --require ./test/test-helper.js
  • corepack yarn dtslint --localTs node_modules/typescript/lib types/
  • corepack yarn eslint -c ./config/.eslintrc.js src/operation.js src/operations/manage_buy_offer.js
  • git diff --check

Copilot AI review requested due to automatic review settings June 1, 2026 14:01
Comment: Adds a manageBuyOffer amount alias while preserving buyAmount compatibility.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds amount as an alias for buyAmount in manageBuyOffer, updating runtime behavior, TypeScript typings, and tests to support the alias.

Changes:

  • Accept opts.amount as an alias of opts.buyAmount when building manageBuyOffer operations.
  • Update TypeScript definitions to allow amount or buyAmount in ManageBuyOffer options.
  • Extend unit tests and XDR decoding output to expose amount alongside buyAmount.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
types/test.ts Adds a TS usage example calling manageBuyOffer with the new amount alias.
types/index.d.ts Updates TS types for ManageBuyOffer options and decoded operation shape.
test/unit/operations/classic_ops_test.js Adds tests for the amount alias and invalid alias type.
src/operations/manage_buy_offer.js Implements amount alias resolution and error attribution.
src/operation.js Adds amount alias on decoded manageBuyOffer operation objects.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +25 to +30
const buyAmount = opts.buyAmount !== undefined ? opts.buyAmount : opts.amount;
const amountArg = opts.buyAmount !== undefined ? 'buyAmount' : 'amount';
if (!this.isValidAmount(buyAmount, true)) {
throw new TypeError(this.constructAmountRequirementsError(amountArg));
}
attributes.buyAmount = this._toXDRAmount(opts.buyAmount);
attributes.buyAmount = this._toXDRAmount(buyAmount);

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57389d5670

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

attributes.buying = opts.buying.toXDRObject();
if (!this.isValidAmount(opts.buyAmount, true)) {
throw new TypeError(this.constructAmountRequirementsError('buyAmount'));
const buyAmount = opts.buyAmount !== undefined ? opts.buyAmount : opts.amount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Respect amount when both aliases are present

When callers start from Operation.fromXDRObject() (which now always returns both buyAmount and amount) and then update the new amount alias before rebuilding a manage-buy-offer, this selection still prefers the stale buyAmount, so the rebuilt XDR silently keeps the old amount. That makes the alias unreliable for the parsed-operation objects this change exposes; either reject conflicting aliases or ensure the alias being edited cannot be shadowed by the legacy field.

Useful? React with 👍 / 👎.

@Ryang-21

Copy link
Copy Markdown
Contributor

Closing with the reasoning listed in the issue this PR was addressing

@Ryang-21 Ryang-21 closed this Jun 23, 2026
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.

manageBuyOffer inconsistent parameter naming

3 participants