From acacd17a90a954496bc5d88e93362a6fdd8c67f1 Mon Sep 17 00:00:00 2001 From: jonathanayubausara-a11y Date: Tue, 28 Jul 2026 18:47:09 +0000 Subject: [PATCH] fix: resolve issues #129, #149, #165 - BatchWallet duplicate handling, Pin Gist API wiring, Terraform backend config --- Frontend/src/api/gists.ts | 64 +++++++ .../src/components/map/AddGistModal.test.tsx | 27 +-- Frontend/src/components/map/AddGistModal.tsx | 17 +- Frontend/src/components/map/Map.tsx | 103 +++++++++-- contracts/batch-wallet/src/lib.rs | 70 +++++-- infrastructure/terraform/backend.tf | 174 ++++++++++++++++++ infrastructure/terraform/providers.tf | 18 +- 7 files changed, 410 insertions(+), 63 deletions(-) create mode 100644 Frontend/src/api/gists.ts create mode 100644 infrastructure/terraform/backend.tf diff --git a/Frontend/src/api/gists.ts b/Frontend/src/api/gists.ts new file mode 100644 index 0000000..7f1506a --- /dev/null +++ b/Frontend/src/api/gists.ts @@ -0,0 +1,64 @@ +// ============================================================================= +// Gists API — typed client for the VertexChain backend POST /gists endpoint. +// ============================================================================= + +export interface GistPayload { + content: string; + lat: number; + lon: number; + author?: string; +} + +export interface GistResponse { + id: string; + content: string; + lat: number; + lon: number; + author: string | null; + created_at: string; + stellar_gist_id?: string | null; +} + +export class GistApiError extends Error { + constructor( + message: string, + public readonly status?: number, + ) { + super(message); + this.name = "GistApiError"; + } +} + +const BASE_URL = + process.env.NEXT_PUBLIC_API_URL || "http://localhost:3000"; + +/** + * POST a new gist to the backend. + * + * Returns the server-confirmed gist on success. + * Throws a {@link GistApiError} on network failure or non-2xx response. + */ +export async function postGist(payload: GistPayload): Promise { + const url = `${BASE_URL}/gists`; + + const res = await fetch(url, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(payload), + }); + + if (!res.ok) { + let body: string; + try { + body = await res.text(); + } catch { + body = "Unable to read response body"; + } + throw new GistApiError( + `POST /gists failed (${res.status}): ${body}`, + res.status, + ); + } + + return res.json() as Promise; +} diff --git a/Frontend/src/components/map/AddGistModal.test.tsx b/Frontend/src/components/map/AddGistModal.test.tsx index 1cefd51..d84851c 100644 --- a/Frontend/src/components/map/AddGistModal.test.tsx +++ b/Frontend/src/components/map/AddGistModal.test.tsx @@ -12,20 +12,15 @@ vi.mock('framer-motion', () => ({ })) const onClose = vi.fn() -const onAddGist = vi.fn() +const onAddGist = vi.fn().mockResolvedValue(undefined) const renderModal = (isOpen = true) => render() describe('AddGistModal', () => { beforeEach(() => { - vi.useFakeTimers() onClose.mockClear() - onAddGist.mockClear() - }) - - afterEach(() => { - vi.useRealTimers() + onAddGist.mockClear().mockResolvedValue(undefined) }) it('does not render the dialog when isOpen is false', () => { @@ -55,28 +50,38 @@ describe('AddGistModal', () => { it('does not call onAddGist when submitted with empty content', () => { renderModal() fireEvent.click(screen.getByRole('button', { name: /pin gist/i })) - act(() => { vi.advanceTimersByTime(2000) }) expect(onAddGist).not.toHaveBeenCalled() }) - it('shows loading state while submitting', () => { + it('shows loading state while submitting', async () => { + // Make onAddGist never resolve so we can observe the loading state + const deferred = new Promise(() => {}) + onAddGist.mockReturnValue(deferred) + renderModal() fireEvent.change(screen.getByRole('textbox', { name: /gist content/i }), { target: { value: 'Traffic jam on the bridge!' }, }) fireEvent.click(screen.getByRole('button', { name: /pin gist/i })) + + // Loading state should appear immediately expect(screen.getByRole('button', { name: /pinning/i })).toBeInTheDocument() expect(screen.getByRole('button', { name: /pinning/i })).toBeDisabled() }) - it('calls onAddGist with the content and then onClose after 2s', () => { + it('calls onAddGist with the content and then onClose on success', async () => { + onAddGist.mockResolvedValue(undefined) + renderModal() fireEvent.change(screen.getByRole('textbox', { name: /gist content/i }), { target: { value: 'Amazing suya spot just opened here!' }, }) fireEvent.click(screen.getByRole('button', { name: /pin gist/i })) - act(() => { vi.advanceTimersByTime(2000) }) + // Wait for the async handler to complete + await act(async () => { + await Promise.resolve() + }) expect(onAddGist).toHaveBeenCalledWith('Amazing suya spot just opened here!') expect(onClose).toHaveBeenCalledTimes(1) diff --git a/Frontend/src/components/map/AddGistModal.tsx b/Frontend/src/components/map/AddGistModal.tsx index 86c4382..ad96b82 100644 --- a/Frontend/src/components/map/AddGistModal.tsx +++ b/Frontend/src/components/map/AddGistModal.tsx @@ -6,7 +6,7 @@ import { motion, AnimatePresence } from 'framer-motion'; interface AddGistModalProps { isOpen: boolean; onClose: () => void; - onAddGist: (content: string) => void; + onAddGist: (content: string) => Promise; } export default function AddGistModal({ @@ -45,17 +45,18 @@ export default function AddGistModal({ return () => document.removeEventListener('keydown', onKey); }, [isOpen, onClose]); - const handleSubmit = useCallback(() => { - if (!content.trim()) return; + const handleSubmit = useCallback(async () => { + if (!content.trim() || isLoading) return; setIsLoading(true); - setTimeout(() => { - onAddGist(content); + try { + await onAddGist(content); setContent(''); - setIsLoading(false); onClose(); - }, 2000); - }, [content, onAddGist, onClose]); + } finally { + setIsLoading(false); + } + }, [content, isLoading, onAddGist, onClose]); return ( diff --git a/Frontend/src/components/map/Map.tsx b/Frontend/src/components/map/Map.tsx index 39230c3..5cf0f74 100644 --- a/Frontend/src/components/map/Map.tsx +++ b/Frontend/src/components/map/Map.tsx @@ -4,16 +4,18 @@ import { MapContainer, TileLayer, Marker, Popup, useMap } from 'react-leaflet'; import 'leaflet/dist/leaflet.css'; import '@/styles/leaflet-dark.css'; import { icon } from 'leaflet'; -import { useState, useEffect } from 'react'; +import { useState, useEffect, useCallback } from 'react'; import { Plus } from 'lucide-react'; import AddGistModal from './AddGistModal'; -import { motion } from 'framer-motion'; +import { motion, AnimatePresence } from 'framer-motion'; +import { postGist, GistApiError } from '@/api/gists'; export interface Gist { - id: number; + id: string | number; content: string; lat: number; lng: number; + status?: 'pending' | 'delivered' | 'error'; } const greenIcon = icon({ @@ -86,15 +88,72 @@ export default function Map() { ); }, []); - const handleAddGist = (content: string) => { - const newGist: Gist = { - id: Date.now(), - content, - lat: position[0], - lng: position[1], - }; - setGists((prevGists) => [...prevGists, newGist]); - }; + const [toast, setToast] = useState<{ + message: string; + type: 'success' | 'error'; + } | null>(null); + + const showToast = useCallback( + (message: string, type: 'success' | 'error') => { + setToast({ message, type }); + setTimeout(() => setToast(null), 4000); + }, + [], + ); + + const handleAddGist = useCallback( + async (content: string) => { + // Create an optimistic gist with a temporary client-side id + const tempId = `optimistic-${Date.now()}`; + const optimisticGist: Gist = { + id: tempId, + content, + lat: position[0], + lng: position[1], + status: 'pending', + }; + + // Optimistic insert — add immediately to local state + setGists((prevGists) => [...prevGists, optimisticGist]); + + try { + const serverGist = await postGist({ + content, + lat: position[0], + lon: position[1], + }); + + // Replace the optimistic entry with the server-confirmed one + setGists((prevGists) => + prevGists.map((g) => + g.id === tempId + ? { + id: serverGist.id, + content: serverGist.content, + lat: serverGist.lat, + lng: serverGist.lon, + status: 'delivered' as const, + } + : g, + ), + ); + + showToast('Gist pinned successfully!', 'success'); + } catch (err) { + // Rollback — remove the optimistic entry on error + setGists((prevGists) => + prevGists.filter((g) => g.id !== tempId), + ); + + const message = + err instanceof GistApiError + ? err.message + : 'Network error — please try again.'; + showToast(message, 'error'); + } + }, + [position, showToast], + ); return (
@@ -117,6 +176,26 @@ export default function Map() { ))} + {/* Toast notification */} + + {toast && ( + + {toast.message} + + )} + + setIsModalOpen(true)} // 3. Animation props for pulsing effect diff --git a/contracts/batch-wallet/src/lib.rs b/contracts/batch-wallet/src/lib.rs index 60f7f5a..05e292f 100644 --- a/contracts/batch-wallet/src/lib.rs +++ b/contracts/batch-wallet/src/lib.rs @@ -224,26 +224,36 @@ impl BatchWalletContract { let mut successful: u32 = 0; let mut failed: u32 = 0; - // Check for duplicates within the batch + // Track addresses seen in *this batch* to handle duplicates gracefully let mut seen_addresses = Vec::new(&env); + + // Process each request — duplicates within the batch yield + // WalletCreateResult::Failure(owner, DuplicateWallet) instead of + // panicking, so the rest of the batch can still succeed. for i in 0..requests.len() { let request = requests.get(i).unwrap(); let owner = request.owner.clone(); - // Check if this address was already seen in this batch + // Check for duplicate within the batch + let mut is_duplicate = false; for seen in seen_addresses.iter() { if seen == owner { - BatchWalletEvents::wallet_duplicate(&env, &owner); - panic_with_error!(&env, BatchWalletError::DuplicateWallet); + is_duplicate = true; + break; } } - seen_addresses.push_back(owner); - } - // Process each request - for i in 0..requests.len() { - let request = requests.get(i).unwrap(); - let owner = request.owner.clone(); + if is_duplicate { + BatchWalletEvents::wallet_duplicate(&env, &owner); + results.push_back(WalletCreateResult::Failure( + owner, + BatchWalletError::DuplicateWallet as u32, + )); + failed += 1; + continue; + } + + seen_addresses.push_back(owner.clone()); if let Some(_existing_wallet) = get_wallet(&env, owner.clone()) { results.push_back(WalletCreateResult::Failure( @@ -503,20 +513,46 @@ mod tests { } #[test] - // BatchWalletError::DuplicateWallet = 7 (`#[repr(u32)]`) — keep in sync if enum is reordered. - #[should_panic(expected = "Error(Contract, #7)")] - fn test_batch_create_wallets_duplicate_in_batch() { + fn test_batch_create_wallets_partial_success_with_duplicates() { + // Send N+1 requests where M are duplicates → result reports failed=M, + // others succeed, event wallet_duplicate emitted per duplicate. let (env, admin, client) = setup_test_env(); - let owner = Address::generate(&env); + let owner1 = Address::generate(&env); let owner2 = Address::generate(&env); + let owner3 = Address::generate(&env); + // 5 requests: owner1, owner2, owner1 (duplicate), owner3, owner2 (duplicate) let mut requests: Vec = Vec::new(&env); - requests.push_back(create_wallet_request(&env, owner.clone())); + requests.push_back(create_wallet_request(&env, owner1.clone())); requests.push_back(create_wallet_request(&env, owner2.clone())); - requests.push_back(create_wallet_request(&env, owner.clone())); + requests.push_back(create_wallet_request(&env, owner1.clone())); // duplicate + requests.push_back(create_wallet_request(&env, owner3.clone())); + requests.push_back(create_wallet_request(&env, owner2.clone())); // duplicate + + let result = client.batch_create_wallets(&admin, &requests); - client.batch_create_wallets(&admin, &requests); + // 5 total, 3 unique → 3 succeed, 2 duplicates fail + assert_eq!(result.total_requests, 5); + assert_eq!(result.successful, 3); + assert_eq!(result.failed, 2); + assert_eq!(result.results.len(), 5); + + // Verify wallets were created for unique owners only + assert!(client.get_wallet(&owner1).is_some()); + assert!(client.get_wallet(&owner2).is_some()); + assert!(client.get_wallet(&owner3).is_some()); + + // Verify duplicate entries are Failure with DuplicateWallet code (7) + let mut duplicate_count = 0; + for r in result.results.iter() { + if let WalletCreateResult::Failure(_, code) = r { + if code == BatchWalletError::DuplicateWallet as u32 { + duplicate_count += 1; + } + } + } + assert_eq!(duplicate_count, 2); } #[test] diff --git a/infrastructure/terraform/backend.tf b/infrastructure/terraform/backend.tf new file mode 100644 index 0000000..1b65d81 --- /dev/null +++ b/infrastructure/terraform/backend.tf @@ -0,0 +1,174 @@ +# ============================================================================= +# Terraform Remote State Backend +# +# Uses S3 for state storage with KMS encryption, DynamoDB for state locking to +# prevent concurrent modifications, and lifecycle rules to manage state history. +# +# The S3 bucket and DynamoDB table are created by this configuration via a +# bootstrap process. After initial creation, uncomment the backend block below +# and run `terraform init -migrate-state` to migrate local state to the remote +# backend. +# ============================================================================= + +# --------------------------------------------------------------------------- +# 1. TERRAFORM BACKEND — uncomment after bootstrap +# --------------------------------------------------------------------------- +# terraform { +# backend "s3" { +# bucket = aws_s3_bucket.terraform_state.bucket +# key = "vertexchain/terraform.tfstate" +# region = var.region +# encrypt = true +# kms_key_id = aws_kms_key.terraform_state.arn +# dynamodb_table = aws_dynamodb_table.terraform_state_lock.name +# +# # Workspace isolation — state paths become: +# # env:/dev/vertexchain/terraform.tfstate +# # env:/staging/vertexchain/terraform.tfstate +# # env:/prod/vertexchain/terraform.tfstate +# workspace_key_prefix = "env:" +# } +# } + +# --------------------------------------------------------------------------- +# 2. KMS KEY — encrypts the Terraform state at rest +# --------------------------------------------------------------------------- +resource "aws_kms_key" "terraform_state" { + description = "KMS key for encrypting Terraform remote state in S3" + deletion_window_in_days = 30 + enable_key_rotation = true + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Sid = "Enable IAM User Permissions" + Effect = "Allow" + Principal = { + AWS = "arn:aws:iam::${data.aws_caller_identity.state_backend.account_id}:root" + } + Action = "kms:*" + Resource = "*" + }, + { + Sid = "Allow S3 to use the key for state encryption" + Effect = "Allow" + Principal = { + Service = "s3.amazonaws.com" + } + Action = [ + "kms:GenerateDataKey", + "kms:Decrypt" + ] + Resource = "*" + } + ] + }) + + tags = local.common_tags +} + +resource "aws_kms_alias" "terraform_state" { + name = "alias/${local.name_prefix}-terraform-state" + target_key_id = aws_kms_key.terraform_state.key_id +} + +# --------------------------------------------------------------------------- +# 3. S3 BUCKET — stores Terraform state files +# --------------------------------------------------------------------------- +resource "aws_s3_bucket" "terraform_state" { + bucket = "${var.project_name}-terraform-state-${data.aws_caller_identity.state_backend.account_id}" + + tags = merge(local.common_tags, { + Purpose = "terraform-remote-state" + }) +} + +# Block all public access +resource "aws_s3_bucket_public_access_block" "terraform_state" { + bucket = aws_s3_bucket.terraform_state.id + block_public_acls = true + block_public_policy = true + ignore_public_acls = true + restrict_public_buckets = true +} + +# Enable versioning so every state change is recoverable +resource "aws_s3_bucket_versioning" "terraform_state" { + bucket = aws_s3_bucket.terraform_state.id + versioning_configuration { + status = "Enabled" + } +} + +# KMS server-side encryption (default for all objects) +resource "aws_s3_bucket_server_side_encryption_configuration" "terraform_state" { + bucket = aws_s3_bucket.terraform_state.id + + rule { + apply_server_side_encryption_by_default { + sse_algorithm = "aws:kms" + kms_master_key_id = aws_kms_key.terraform_state.arn + } + bucket_key_enabled = true + } +} + +# --------------------------------------------------------------------------- +# 4. LIFECYCLE RULES — manage state file history +# - Keep the latest N noncurrent versions for rollback +# - Expire older versions after 90 days +# --------------------------------------------------------------------------- +resource "aws_s3_bucket_lifecycle_configuration" "terraform_state" { + bucket = aws_s3_bucket.terraform_state.id + + rule { + id = "state-version-lifecycle" + status = "Enabled" + + # Keep up to 10 noncurrent versions for safe rollback + noncurrent_version_expiration { + noncurrent_days = 90 + newer_noncurrent_versions = 10 + } + + # Abort incomplete multipart uploads (e.g. from interrupted applies) + abort_incomplete_multipart_upload { + days_after_initiation = 7 + } + } +} + +# --------------------------------------------------------------------------- +# 5. DYNAMODB TABLE — state locking to prevent concurrent modifications +# --------------------------------------------------------------------------- +resource "aws_dynamodb_table" "terraform_state_lock" { + name = "${var.project_name}-terraform-state-locks" + billing_mode = "PAY_PER_REQUEST" + hash_key = "LockID" + + attribute { + name = "LockID" + type = "S" + } + + # Enable point-in-time recovery so lock history is preserved + point_in_time_recovery { + enabled = true + } + + server_side_encryption { + enabled = true + } + + tags = merge(local.common_tags, { + Purpose = "terraform-state-lock" + }) +} + +# --------------------------------------------------------------------------- +# 6. DATA SOURCE — needed for KMS policy and globally unique bucket name. +# Uses a distinct name ("state_backend") to avoid colliding with the +# conditional data "aws_caller_identity" "current" in backup-vaults.tf. +# --------------------------------------------------------------------------- +data "aws_caller_identity" "state_backend" {} diff --git a/infrastructure/terraform/providers.tf b/infrastructure/terraform/providers.tf index fccdb70..d23872e 100644 --- a/infrastructure/terraform/providers.tf +++ b/infrastructure/terraform/providers.tf @@ -1,21 +1,9 @@ terraform { required_version = ">= 1.5.0" - # The S3 backend automatically isolates state per workspace when using - # workspace_key_prefix. State paths will be: - # env:/dev/vertexchain/terraform.tfstate - # env:/staging/vertexchain/terraform.tfstate - # env:/prod/vertexchain/terraform.tfstate - # The default workspace continues to use the key directly: - # vertexchain/terraform.tfstate - backend "s3" { - bucket = "vertexchain-terraform-state" - key = "vertexchain/terraform.tfstate" - workspace_key_prefix = "env:" - region = "us-east-1" - encrypt = true - dynamodb_table = "vertexchain-terraform-locks" - } + # Backend configuration has been moved to backend.tf. + # See backend.tf for S3 remote state, DynamoDB locking, + # KMS encryption, and lifecycle rules. required_providers { aws = {