Skip to content

docs: added missing docs#144

Merged
jopmiddelkamp merged 1 commit intomasterfrom
chore/update-docs
Mar 31, 2026
Merged

docs: added missing docs#144
jopmiddelkamp merged 1 commit intomasterfrom
chore/update-docs

Conversation

@jopmiddelkamp
Copy link
Copy Markdown
Contributor

Auto-generated Summary 🤖

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Copy Markdown
Contributor Author

@jopmiddelkamp jopmiddelkamp left a comment

Choose a reason for hiding this comment

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

📋 Documentation Accuracy Review

I reviewed all documentation changes in this PR for factual correctness by cross-referencing against:

Found 15 factual errors across the PR. Grouped by severity below.

Overall, the vast majority of the ~4800 lines of documentation added are accurate. These 15 issues are the exceptions that should be corrected before merging.

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 1/15: wasmHash described as "Wasm bytes" instead of "hash of Wasm bytes"

File: StellarDotnetSdk/Operations/InvokeHostFunctionOperation.cs — lines 164, 417, 490
Also in: StellarDotnetSdk/LedgerKeys/LedgerKeyContractCode.cs — line 16

Current doc:

A hex-encoded string of the Wasm bytes of a compiled smart contract.

Should be:

A hex-encoded hash (SHA-256) of the Wasm bytecode of a compiled smart contract.

Proof:

  • The XDR schema defines the type as Hash wasm_hash — the field type is Hash, not opaque bytes
  • The property is named WasmHash (not WasmBytes)
  • A WASM module is thousands of bytes; this value is always 32 bytes (a SHA-256 digest)
  • The code constructs new Hash(Convert.FromHexString(WasmHash)) — wrapping it in a Hash XDR type
  • The Flutter SDK correctly documents this as "Hex-encoded hash of the uploaded WASM code"
  • The constructor doc on line 2028 of SCVal.cs even correctly says "A hex-encoded hash of the compiled Wasm module" — contradicting the property doc

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 2/15: Salt described as "for the token ID" instead of "for the contract ID"

File: StellarDotnetSdk/Operations/InvokeHostFunctionOperation.cs — line 168

Current doc:

(Optional) Custom salt 32-byte salt for the token ID. It will be randomly generated if omitted.

Should be:

(Optional) A custom 32-byte salt for deriving the contract ID. It will be randomly generated if omitted.

Proof:

  • The XDR uses ContractIDPreimage — the salt is part of deriving the contract ID, not a "token ID"
  • The accountId param doc on line 166 in this same method correctly says "The address to use to derive the contract ID"
  • "Token ID" is not a Stellar concept in this context
  • Also note the minor word duplication: "Custom salt 32-byte salt"

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 3/15: NumberSponsored and NumberSponsoring descriptions are SWAPPED

File: StellarDotnetSdk/LedgerEntries/AccountEntryExtensionV2.cs — lines 19 and 24

Current doc:

/// The number of entries this account is sponsoring (reserves are being paid by this account).
public uint NumberSponsored { get; }

/// The number of entries sponsoring this account (reserves paid by other accounts).
public uint NumberSponsoring { get; }

Should be (swap the descriptions):

/// The number of entries sponsored for this account (reserves paid by other accounts).
public uint NumberSponsored { get; }

/// The number of entries this account is sponsoring (reserves this account pays for others).
public uint NumberSponsoring { get; }

Proof:

  • CAP-0033 (Sponsored Reserves) defines:
    • numSponsored = entries sponsored for this account (reduces reserve requirement)
    • numSponsoring = entries this account is sponsoring (increases reserve requirement)
  • This SDK's own AccountResponse.cs correctly documents them the right way around

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 4/15: Offer price formula is inverted

File: StellarDotnetSdk/LedgerEntries/LedgerEntryOffer.cs — line 57

Current doc:

The price as a ratio of selling to buying: price = selling / buying.

Should be:

The price as a ratio of buying to selling: price = buying / selling.

Proof:

  • The Stellar XDR OfferEntry definition explicitly states:
    /* price for this offer:
        price of A in terms of B
        price=AmountB/AmountA=priceNumerator/priceDenominator
    */
    
    Where A = selling asset and B = buying asset, so price = buying / selling

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 5/15: ManageBuyOfferBuyNotAuthorized and SellNotAuthorized descriptions are SWAPPED

File: StellarDotnetSdk/Responses/Results/ManageBuyOfferResult.cs — lines 171 and 176

Current doc:

/// The account creating the offer is not authorized to sell this asset.
public class ManageBuyOfferBuyNotAuthorized : ManageBuyOfferResult;

/// The account creating the offer is not authorized to buy this asset.
public class ManageBuyOfferSellNotAuthorized : ManageBuyOfferResult;

Should be (swap the descriptions):

/// The account creating the offer is not authorized to buy this asset.
public class ManageBuyOfferBuyNotAuthorized : ManageBuyOfferResult;

/// The account creating the offer is not authorized to sell this asset.
public class ManageBuyOfferSellNotAuthorized : ManageBuyOfferResult;

Proof:

  • The XDR in ManageBuyOfferResultCode.cs defines:
    • MANAGE_BUY_OFFER_SELL_NOT_AUTHORIZED = -4 → not authorized to sell
    • MANAGE_BUY_OFFER_BUY_NOT_AUTHORIZED = -5 → not authorized to buy
  • For comparison, ManageSellOfferResult.cs has them the correct way around

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 6/15: OfferEntryFlags.PASSIVE has completely wrong description

File: StellarDotnetSdk/Responses/Results/OfferEntry.cs — line 20

Current doc:

/// Issuer has authorized account to perform transactions with its credit.
PASSIVE = 1,

Should be:

/// An offer with this flag will not act on and take a reverse offer of equal price.
PASSIVE = 1,

Proof:

  • The current description belongs to AUTH_REQUIRED_FLAG from AccountFlags — it was copy-pasted from a completely different enum
  • The XDR in OfferEntryFlags.cs says:
    // an offer with this flag will not act on and take a reverse offer of equal price
    // PASSIVE_FLAG = 1
    
  • This is about passive sell offers which cannot be matched against a crossing offer at the same price

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 7/15: TransactionResult.FromXdrBase64 return doc says "LedgerEntry" instead of "TransactionResult"

File: StellarDotnetSdk/Responses/Results/TransactionResult.cs — line 37

Current doc:

<returns>LedgerEntry object</returns>

Should be:

<returns>TransactionResult object</returns>

Proof:

  • The method signature on line 38 is public static TransactionResult FromXdrBase64(string xdrBase64) — it returns a TransactionResult, not a LedgerEntry

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 8/15: SorobanTransactionData.toExtend has the operations REVERSED

File: StellarDotnetSdk/Soroban/SorobanTransactionData.cs — line 20

Current doc:

True: To be used in an RestoreTtl operation. False: To be used in a ExtendTtl operation.

Should be:

True: To be used in an ExtendFootprintTTL operation. False: To be used in a RestoreFootprint operation.

Proof:

  • The code on lines 24-26 shows:
    • toExtend = true → sets ReadOnly = [key] → used by ExtendFootprintTTL (extends TTL of entries in readOnly footprint)
    • toExtend = false → sets ReadWrite = [key] → used by RestoreFootprint (restores archived entries in readWrite footprint)
  • The operation names are also non-standard: "RestoreTtl" and "ExtendTtl" should be "RestoreFootprint" and "ExtendFootprintTTL" per the Stellar docs

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 9/15: Copy-paste error on clientDomain and clientKeypair params

File: StellarDotnetSdk/Sep/Sep0010/ServerWebAuth.cs — lines 44-45

Current doc:

<param name="clientDomain">The network the transaction will be submitted to</param>
<param name="clientKeypair">The network the transaction will be submitted to</param>

Both descriptions are copy-pasted from the network parameter on line 43 and are factually wrong.

Should be:

<param name="clientDomain">Optional Client Domain</param>
<param name="clientKeypair">Client Signing Key (Used with Client Domain)</param>

Proof:

  • The second overload of this same method (lines 96-97) correctly documents these as "Optional Client Domain" and "Client Signing Key (Used with Client Domain)"
  • The code on lines 117-120, 156-163 uses clientDomain to set a client_domain ManageData operation per SEP-0010

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🔴 Finding 10/15: ClaimPredicate.Or has copy-pasted And description

File: StellarDotnetSdk/Claimants/ClaimPredicate.cs — line 36

Current doc:

/// <summary>Creates a predicate requiring both sub-predicates to be satisfied.</summary>
public static ClaimPredicate Or(...)

Should be:

/// <summary>Creates a predicate requiring at least one sub-predicate to be satisfied.</summary>
public static ClaimPredicate Or(...)

Proof:

  • The method returns new ClaimPredicateOr(...) (line 42)
  • The ClaimPredicateOr class doc correctly says "either the left or right predicate (or both) is satisfied"
  • The And method on line 45 has the identical text "requiring both sub-predicates" — this was clearly copy-pasted

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🟡 Finding 11/15: LedgerBounds.MaxLedger — upper bound is exclusive, not inclusive

File: StellarDotnetSdk/Transactions/LedgerBounds.cs — line 29

Current doc:

The transaction is valid only at or before this ledger.

Should be:

The transaction is valid only before this ledger (exclusive upper bound).

Proof:

  • CAP-0021 (Generalized Transaction Preconditions) defines: minLedger <= n < maxLedger — the upper bound is exclusive
  • stellar-core implements isTooLate as maxLedger <= currentLedger, meaning the transaction is only valid when currentLedger < maxLedger
  • The official Stellar docs state: "The lower bound is inclusive (less than or equal to) while the upper bound is not (just greater than)"

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🟡 Finding 12/15: ManageDataOperation name limit is "64 bytes", not "64 characters"

File: StellarDotnetSdk/Operations/ManageDataOperation.cs — line 48

Current doc:

The name of the data entry (up to 64 characters).

Should be:

The name of the data entry (up to 64 bytes).

Proof:

  • The Stellar docs say "String up to 64 bytes long" for the name parameter
  • The XDR typedef string string64<64> is a byte limit, not character limit
  • Multi-byte UTF-8 characters can cause a string under 64 characters to exceed 64 bytes
  • The pre-existing doc on the first constructor (line 23 of the same file) correctly says "string up to 64 bytes long" — the new doc is internally inconsistent

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🟡 Finding 13/15: ReadChallengeTransaction says "one operation only" but spec requires "at least one"

File: StellarDotnetSdk/Sep/Sep0010/ServerWebAuth.cs — lines 177 and 212

Current doc:

3. Transaction has one operation only, of type ManageDataOperation

Should be:

3. Transaction has at least one operation, of type ManageDataOperation

Proof:

  • SEP-0010 says: "verify that transaction contains at least one operation"
  • The code at line 372 checks transaction.Operations.Length < 1 with error message "must contain at least one operation"
  • A valid challenge transaction typically has 2+ operations: home domain ManageData + web_auth_domain ManageData, optionally + client_domain ManageData

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🟡 Finding 14/15: now parameter default described incorrectly

File: StellarDotnetSdk/Sep/Sep0010/ServerWebAuth.cs — line 222

Current doc:

<param name="now">Current time, defaults to DateTimeOffset.Now + GracePeriod</param>

Should be:

<param name="now">Current time, defaults to DateTimeOffset.UtcNow</param>

Proof:

  • Line 234 shows: now ??= DateTimeOffset.UtcNow; — the default is just DateTimeOffset.UtcNow, with no grace period added
  • The grace period is applied separately in ValidateTimeBounds by extending the time bounds window, not by adjusting now
  • Also uses DateTimeOffset.Now (local time) instead of DateTimeOffset.UtcNow

@jopmiddelkamp
Copy link
Copy Markdown
Contributor Author

🟢 Finding 15/15: bank_account_type — "e.g." implies other values exist

File: StellarDotnetSdk/Sep/Sep0009/FinancialAccountKycFields.cs — line 19

Current doc:

Field key for the bank_account_type field (e.g., checking or savings) as defined in SEP-0009.

Should be:

Field key for the bank_account_type field (checking or savings) as defined in SEP-0009.

Proof:

  • SEP-0009 defines bank_account_type as exactly `checking` or `savings` — these are the defined values, not examples
  • Using "e.g." implies there are other possible values, which misrepresents the spec

@jopmiddelkamp jopmiddelkamp merged commit 0414518 into master Mar 31, 2026
4 checks passed
@jopmiddelkamp jopmiddelkamp deleted the chore/update-docs branch March 31, 2026 04:14
@cuongph87 cuongph87 added the documentation Improvements or additions to documentation label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants