Repository navigation
Description
Overview
Every DTO field representing a Stellar public key — funderAddress on FundEscrowDto, DepositDto, and the inline FundBountyDto/FundMilestoneDto; recipientAddress on ReleaseEscrowDto, SplitRecipientDto, and AssignRewardDto — is validated with nothing more than @IsString():
// src/escrow/dto/fund-escrow.dto.ts:18-20
@ApiProperty({ description: 'Stellar public key of the funding sponsor' })
@IsString()
funderAddress: string;// src/escrow/dto/release-escrow.dto.ts:4-7
@ApiProperty({ description: 'Stellar public key of the recipient' })
@IsString()
recipientAddress: string;A real Stellar public key ("StrKey") is a specific, checkable format: it starts with G, is exactly 56 characters, is valid base32, and encodes a version byte plus a CRC16 checksum over the payload — the @stellar/stellar-sdk package already depended on by this codebase exposes StrKey.isValidEd25519PublicKey(string) to validate exactly this, for free, with no new dependency needed. None of these DTOs use it, or any other format check — @IsString() accepts "", "not-an-address", a 55-character near-miss, or a syntactically-plausible-but-checksum-invalid string with equal enthusiasm.
Trace what happens to a garbage address that gets past validation. It reaches SorobanClientService.toScVal's heuristic:
// src/escrow/soroban-client.service.ts:167-176
private toScVal(value: unknown) {
if (typeof value === 'string' && value.length >= 32 && /^[A-Z0-9]+$/.test(value)) {
try { return new Address(value).toScVal(); }
catch { return nativeToScVal(value, { type: 'string' }); } // <- silent fallback, not a rejection
}
// ...
}An invalid-but-address-shaped string (56 uppercase alphanumeric characters that fail the SDK's checksum) throws inside new Address(value), which is caught and silently downgraded to a generic string encoding — not surfaced as a validation error anywhere. An invalid-and-not-address-shaped string (wrong length, lowercase, whatever) never even reaches the Address constructor — it goes straight to the generic string fallback. Either way, nothing in this pipeline ever produces a clean 400 Bad Request telling the caller "this isn't a valid Stellar address." Whatever eventually happens next depends entirely on how the deployed Soroban contract's Rust code handles being given a malformed Address argument — likely a confusing simulation failure deep inside soroban.invoke (per the companion "no BytesN<32> encoding" issue, this whole path is untested against a real contract today), or, in the worst case, an amount genuinely getting "released" toward an address that was never checked to be a real, spendable Stellar account, silently consuming the escrow's LOCKED funds with nothing to show for it.
This is explicitly not the same gap as the already-closed "amount fields accept unbounded string input" issue in this repo — that issue hardened IsMoneyAmount/isSupportedEscrowAsset for numeric/asset fields specifically and doesn't mention address fields anywhere. It's also distinct from the already-open "Bounty payout to a contributor with no linked Stellar address silently releases to an empty-string recipient" issue — that issue is about a missing address (the ?? '' fallback when stellarAddress is null); this issue is about an address that was provided but was never checked to be syntactically or cryptographically valid in the first place, whether it arrives via the empty-string path or a directly client-supplied funderAddress/recipientAddress on the raw escrow/pool endpoints.
Requirements
- Add a reusable
@IsStellarAddress()class-validator decorator (mirroring the existingIsMoneyAmount/IsSupportedEscrowAssetpattern insrc/common/validators/money.validator.ts, perhaps in a siblingstellar-address.validator.ts) backed byStrKey.isValidEd25519PublicKeyfrom@stellar/stellar-sdk. - Apply it to every
funderAddress/recipientAddressfield currently typed as a bare@IsString():FundEscrowDto.funderAddress,DepositDto.funderAddress,ReleaseEscrowDto.recipientAddress,SplitRecipientDto.recipientAddress,AssignRewardDto.recipientAddress, and the inlineFundBountyDto.funderAddress/FundMilestoneDto.funderAddressin the bounties/milestones controllers. - Ensure the empty-string fallback from the companion "no linked Stellar address" issue is also caught by this same validator once that issue's fix routes an address through these DTOs/service methods — an empty string should fail
IsStellarAddress()too, giving that issue's fix a second, independent layer of protection rather than relying solely on whatever check that issue's own fix adds. - Add tests: a syntactically-invalid string, a checksum-invalid-but-right-length string, and an empty string are all rejected at the DTO layer with a
400, before ever reachingEscrowService/SorobanClientService.
Acceptance Criteria
- A reusable Stellar-address validator exists and is backed by the SDK's own
StrKeychecksum validation, not a hand-rolled regex. - Every DTO field listed above uses it.
- A malformed address (wrong length, wrong prefix, invalid checksum, or empty) is rejected with a
400at the API boundary, proven by a test that feeds each case directly to the HTTP layer (not just unit-testing the validator function in isolation, matching the rigor of the existing amount-validation fix's own acceptance criteria). -
SorobanClientService.toScVal's silent try/catch fallback for an unparseable address (:172-175) is no longer the only thing standing between a malformed address and a Soroban call — by the time execution reaches this function, the address has already been validated.
Additional Notes
Precise references: src/escrow/dto/fund-escrow.dto.ts:18-20, src/escrow/dto/release-escrow.dto.ts:4-7, src/escrow/dto/split-release.dto.ts:14-17, src/maintenance-pool/dto/create-pool.dto.ts and the inline DepositDto/AssignRewardDto in src/maintenance-pool/maintenance-pool.controller.ts:9-27, the inline FundBountyDto in src/bounties/bounties.controller.ts:10-13, the inline FundMilestoneDto/ResolveIssueDto in src/milestones/milestones.controller.ts:8-20 — every one of these currently uses bare @IsString() for a field that's supposed to be a Stellar public key. src/escrow/soroban-client.service.ts:167-176 (toScVal's silent fallback, the downstream consequence). src/common/validators/money.validator.ts (the existing, already-closed amount-validation fix this issue's requested decorator should sit alongside, following the same IsXyz()/isValidXyz() naming and structure).
Test/reproduction plan:
const cases = ['', 'not-an-address', 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA', /* right length, bad checksum */];
for (const bad of cases) {
await request(app).post('/escrow/fund')
.send({ amount: '10.0000000', asset: 'USDC', funderAddress: bad, bountyId })
.expect(400);
}
const validCases = [Keypair.random().publicKey()];
for (const good of validCases) {
await request(app).post('/escrow/fund').send({ ...validPayload, funderAddress: good }).expect(201);
}Cross-references: distinct from, and complementary to, the closed "amount fields accept unbounded string input" issue (same class of gap — DTO-layer validation missing for a financially-critical field type — applied to the address surface that issue didn't cover) and the open "Bounty payout to a contributor with no linked Stellar address silently releases to an empty-string recipient" issue (that issue is about a missing address specifically in the merge-and-release flow; this issue is the general-purpose format/checksum validation that should exist regardless of how an invalid address value arrives, including but not limited to that empty-string path). Also relevant to the companion "no BytesN<32> encoding" issue, since a validated, guaranteed-well-formed address is one less variable to account for once that encoding work is undertaken.
Activity
ghzhost commentedon Aug 16, 2026
I have implemented the fix and opened PR #66 with full test coverage and validation across all Stellar address DTOs and EscrowService methods.
kosisochukwu1234 commentedon Aug 16, 2026
Hello, I’ve gone through the requirements and I’m confident I can implement a proper solution that matches the acceptance criteria. Kindly assign this issue to me.
oluwarantimini commentedon Aug 16, 2026
The API DTOs are not actually validating that Stellar address fields contain valid Stellar public keys. Every
funderAddressandrecipientAddressfield is protected only by@IsString(), which verifies the JavaScript type but says nothing about the address format.A Stellar StrKey public key has a specific encoded structure, including the expected prefix, length, base32 representation, version byte, and checksum. The project already depends on
@stellar/stellar-sdk, which providesStrKey.isValidEd25519PublicKey()specifically for this validation, so there is no need to introduce another dependency.With the current validation, values such as an empty string, arbitrary text, incorrectly sized keys, or checksum-invalid StrKeys pass DTO validation and reach deeper application logic before failing—or potentially being stored or processed.
I have experience reviewing API validation boundaries where blockchain-specific identifiers need stronger validation than generic string checks. These DTOs should validate public-key fields with the SDK's StrKey validator (via a custom
class-validatorconstraint or equivalent) so malformed addresses are rejected at the API boundary with a clear validation error.grantfox-oss commentedon Aug 16, 2026
🦊 GrantFox — @oluwarantimini has been assigned to this issue as part of the Third Campaign campaign!
Next steps:
Closes #60)Good luck! Track your progress on GrantFox.
Add IsStellarAddress validator backed by StrKey checksum validation (#60
Apply IsStellarAddress to FundEscrowDto.funderAddress (#60)
Apply IsStellarAddress to ReleaseEscrowDto.recipientAddress (#60)
Apply IsStellarAddress to SplitRecipientDto.recipientAddress (#60)
Apply IsStellarAddress to DepositDto/AssignRewardDto address fields (#60
Apply IsStellarAddress to FundBountyDto.funderAddress (#60)
Apply IsStellarAddress to FundMilestoneDto.funderAddress (#60)
grantfox-oss commentedon Aug 19, 2026
🎉 This issue has been marked as completed on GrantFox as part of the Third Campaign campaign!
@oluwarantimini's PR #84 was approved and merged by @chonilius.
🏆 @oluwarantimini: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (118 total points). Track your full progress on GrantFox.
👏 Great work, @oluwarantimini! Keep contributing to MergeFi.
Apply IsStellarAddress to ResolveIssueDto.recipientAddress (#60)
Add e2e tests: malformed address rejected at HTTP boundary for escrow (…
Add e2e tests: malformed address rejected at HTTP boundary for mainte…
Add e2e tests: malformed address rejected at HTTP boundary for bounti…
test(MergeFi#60): adapt address-validation e2e specs to the new auth …
feat(validators): add IsStellarAddress decorator using StrKey validat…
feat(escrow): apply IsStellarAddress validator to funderAddress (Merg…