Merge pull request #1599 from romeoville/fix/four-issues

feat: implement 4 open source contributions

Author: ritik4ever

contracts/src/lib.rs

@@ -50,10 +50,29 @@ fn guard_ledger_timestamp(env: &Env) -> u64 {
 
 #[contractimpl]
 impl PortfolioRebalancer {
+    /// Validate that `reflector_address` behaves like a Reflector oracle by
+    /// making a lightweight read-only call (`base()`) and checking the result.
+    ///
+    /// This is a best-effort guard, not a full interface conformance check:
+    /// - A malicious contract could implement `base()` but return wrong data in
+    ///   `lastprice()`. Further operations (rebalance, preview) will detect
+    ///   missing or invalid price data at the point of use.
+    /// - The goal is to fail early when the address is clearly not a Reflector
+    ///   contract (e.g. a typo, an EOA, or a random contract).
     pub fn initialize(env: Env, admin: Address, reflector_address: Address) -> Result<(), Error> {
         if env.storage().instance().has(&DataKey::Initialized) {
             return Err(Error::AlreadyInitialized);
         }
+
+        // Lightweight validation: call base() on the provided address.
+        // If the call fails (host error) or returns an unexpected type, the
+        // address is not a valid Reflector oracle.
+        let reflector_client = ReflectorClient::new(&env, &reflector_address);
+        match reflector_client.try_base() {
+            Ok(Ok(_asset)) => {}
+            _ => return Err(Error::InvalidOracleAddress),
+        }
+
         env.storage().instance().set(&DataKey::Admin, &admin);
         env.storage()
             .instance()

contracts/src/test.rs

@@ -170,6 +170,22 @@ mod reflector_without_prices {
     }
 }
 
+// A contract that does NOT implement the Reflector interface.
+// Used to verify that initialize() rejects non-conforming addresses.
+mod non_reflector_contract {
+    use soroban_sdk::{contract, contractimpl, Env};
+
+    #[contract]
+    pub struct NonReflector;
+
+    #[contractimpl]
+    impl NonReflector {
+        pub fn hello(_env: Env) -> bool {
+            true
+        }
+    }
+}
+
 #[test]
 fn test_create_portfolio() {
     let env = Env::default();
@@ -1145,6 +1161,32 @@ fn test_initialize_guard() {
     client.initialize(&admin, &reflector_id);
 }
 
+#[test]
+fn test_initialize_rejects_invalid_reflector_address() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register_contract(None, PortfolioRebalancer);
+    let client = PortfolioRebalancerClient::new(&env, &contract_id);
+    let non_reflector_id = env.register_contract(None, non_reflector_contract::NonReflector);
+    let admin = Address::generate(&env);
+
+    let result = client.try_initialize(&admin, &non_reflector_id);
+    assert_eq!(result, Err(Ok(Error::InvalidOracleAddress)));
+}
+
+#[test]
+fn test_initialize_accepts_valid_reflector_address() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register_contract(None, PortfolioRebalancer);
+    let client = PortfolioRebalancerClient::new(&env, &contract_id);
+    let reflector_id = env.register_contract(None, reflector_contract::MockReflector);
+    let admin = Address::generate(&env);
+
+    let result = client.try_initialize(&admin, &reflector_id);
+    assert_eq!(result, Ok(Ok(())));
+}
+
 #[test]
 #[should_panic]
 fn test_create_portfolio_invalid_allocation() {

contracts/src/types.rs

@@ -189,6 +189,7 @@ pub enum Error {
     InvalidAmount = 26,
     WithdrawFailed = 27,
     InvalidAllocationSum = 28,
+    InvalidOracleAddress = 29,
 }
 
 #[contracttype]

frontend/src/components/ErrorBoundary.test.tsx

@@ -0,0 +1,120 @@
+import { describe, it, expect, vi, afterEach } from 'vitest'
+import { render, screen, cleanup } from '@testing-library/react'
+
+vi.mock('../observability', () => ({
+    Sentry: {
+        captureException: vi.fn(),
+    },
+}))
+
+vi.mock('../utils/walletManager', () => ({
+    walletManager: {
+        getPublicKey: vi.fn(() => null),
+    },
+}))
+
+import { ErrorBoundary } from './ErrorBoundary'
+import { Sentry } from '../observability'
+
+function Bomb() {
+    throw new Error('Test render error')
+}
+
+function Safe() {
+    return <div>Safe content</div>
+}
+
+describe('ErrorBoundary', () => {
+    afterEach(() => {
+        cleanup()
+        vi.clearAllMocks()
+    })
+
+    it('renders children when no error occurs', () => {
+        render(
+            <ErrorBoundary>
+                <Safe />
+            </ErrorBoundary>,
+        )
+        expect(screen.getByText('Safe content')).toBeInTheDocument()
+    })
+
+    it('renders fallback UI when a child throws', () => {
+        const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+
+        render(
+            <ErrorBoundary>
+                <Bomb />
+            </ErrorBoundary>,
+        )
+
+        expect(screen.getByText(/section error/i)).toBeInTheDocument()
+        expect(screen.getByText(/something went wrong/i)).toBeInTheDocument()
+
+        consoleError.mockRestore()
+    })
+
+    it('reports the error to Sentry with context', () => {
+        const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+
+        render(
+            <ErrorBoundary fallbackTitle="PortfolioSection">
+                <Bomb />
+            </ErrorBoundary>,
+        )
+
+        expect(Sentry.captureException).toHaveBeenCalledTimes(1)
+        expect(Sentry.captureException).toHaveBeenCalledWith(
+            expect.any(Error),
+            expect.objectContaining({
+                extra: expect.objectContaining({
+                    componentStack: expect.any(String),
+                    section: 'PortfolioSection',
+                }),
+            }),
+        )
+
+        consoleError.mockRestore()
+    })
+
+    it('resets error state on retry', () => {
+        const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+        const onRetry = vi.fn()
+
+        const { rerender } = render(
+            <ErrorBoundary key="first" onRetry={onRetry}>
+                <Bomb />
+            </ErrorBoundary>,
+        )
+
+        expect(screen.getByText(/section error/i)).toBeInTheDocument()
+
+        rerender(
+            <ErrorBoundary key="second" onRetry={onRetry}>
+                <Safe />
+            </ErrorBoundary>,
+        )
+
+        expect(screen.getByText('Safe content')).toBeInTheDocument()
+
+        consoleError.mockRestore()
+    })
+
+    it('calls onRetry when retry button is clicked', () => {
+        const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+        const onRetry = vi.fn()
+
+        render(
+            <ErrorBoundary onRetry={onRetry}>
+                <Bomb />
+            </ErrorBoundary>,
+        )
+
+        const retryButton = screen.getByRole('button', { name: /retry/i })
+        retryButton.click()
+
+        expect(onRetry).toHaveBeenCalledTimes(1)
+
+        consoleError.mockRestore()
+    })
+})

frontend/src/components/ErrorBoundary.tsx

@@ -1,5 +1,7 @@
 import { Component, type ErrorInfo, type ReactNode } from 'react'
 import { RefreshCw } from 'lucide-react'
+import { Sentry } from '../observability'
+import { walletManager } from '../utils/walletManager'
 
 interface Props {
     children: ReactNode
@@ -29,7 +31,25 @@ export class ErrorBoundary extends Component<Props, State> {
     }
 
     componentDidCatch(error: Error, errorInfo: ErrorInfo): void {
-        console.error('[ErrorBoundary] Caught error:', error, errorInfo.componentStack)
+        const route = typeof window !== 'undefined' ? window.location.pathname : undefined
+        let userId: string | undefined
+        try {
+            const pk = walletManager.getPublicKey()
+            if (pk) userId = pk
+        } catch {
+            // walletManager not available
+        }
+        Sentry.captureException(error, {
+            extra: {
+                componentStack: errorInfo.componentStack,
+                route,
+                userId,
+                section: this.props.fallbackTitle,
+            },
+            tags: {
+                errorBoundary: 'section',
+            },
+        })
     }
 
     private handleRetry = (): void => {

frontend/src/components/RouteErrorState.test.tsx

@@ -0,0 +1,149 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+import { render, screen, cleanup, act } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
+import RouteErrorState from './RouteErrorState'
+
+describe('RouteErrorState', () => {
+    beforeEach(() => {
+        vi.useFakeTimers()
+    })
+
+    afterEach(() => {
+        cleanup()
+        vi.restoreAllMocks()
+    })
+
+    it('renders title and message', () => {
+        render(
+            <RouteErrorState
+                title="Loading Failed"
+                message="Could not load portfolio data."
+                onRetry={() => {}}
+            />,
+        )
+        expect(screen.getByText('Loading Failed')).toBeInTheDocument()
+        expect(screen.getByText('Could not load portfolio data.')).toBeInTheDocument()
+    })
+
+    it('calls onRetry when retry button is clicked', async () => {
+        const onRetry = vi.fn()
+        render(
+            <RouteErrorState
+                title="Error"
+                message="Something went wrong."
+                onRetry={onRetry}
+            />,
+        )
+
+        const button = screen.getByRole('button', { name: /retry/i })
+        await act(async () => {
+            button.click()
+        })
+
+        expect(onRetry).not.toHaveBeenCalled()
+
+        act(() => {
+            vi.advanceTimersByTime(1000)
+        })
+
+        expect(onRetry).toHaveBeenCalledTimes(1)
+    })
+
+    it('applies exponential backoff on repeated retries', async () => {
+        const onRetry = vi.fn()
+        render(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={onRetry}
+            />,
+        )
+
+        const button = screen.getByRole('button', { name: /retry/i })
+
+        await act(async () => { button.click() })
+        act(() => { vi.advanceTimersByTime(1000) })
+        expect(onRetry).toHaveBeenCalledTimes(1)
+
+        await act(async () => { button.click() })
+        expect(onRetry).toHaveBeenCalledTimes(1)
+
+        act(() => { vi.advanceTimersByTime(1000) })
+        expect(onRetry).toHaveBeenCalledTimes(1)
+
+        act(() => { vi.advanceTimersByTime(1000) })
+        expect(onRetry).toHaveBeenCalledTimes(2)
+
+        await act(async () => { button.click() })
+        act(() => { vi.advanceTimersByTime(4000) })
+        expect(onRetry).toHaveBeenCalledTimes(3)
+    })
+
+    it('shows loading indicator while retry is in progress', async () => {
+        render(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={() => {}}
+            />,
+        )
+
+        const button = screen.getByRole('button', { name: /retry/i })
+        expect(button).not.toBeDisabled()
+
+        await act(async () => { button.click() })
+
+        expect(button).toBeDisabled()
+        expect(screen.getByText('Retrying…')).toBeInTheDocument()
+    })
+
+    it('renders back button when onBack is provided', () => {
+        render(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={() => {}}
+                onBack={() => {}}
+            />,
+        )
+        expect(screen.getByRole('button', { name: /back/i })).toBeInTheDocument()
+    })
+
+    it('resets retry count after loading completes', async () => {
+        const onRetry = vi.fn()
+        const { rerender } = render(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={onRetry}
+                loading={false}
+            />,
+        )
+
+        const button = screen.getByRole('button', { name: /retry/i })
+
+        await act(async () => { button.click() })
+        act(() => { vi.advanceTimersByTime(1000) })
+        expect(onRetry).toHaveBeenCalledTimes(1)
+
+        rerender(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={onRetry}
+                loading={true}
+            />,
+        )
+
+        rerender(
+            <RouteErrorState
+                title="Error"
+                message="Failed."
+                onRetry={onRetry}
+                loading={false}
+            />,
+        )
+
+        act(() => { vi.advanceTimersByTime(5000) })
+    })
+})