From 86d72c80c69c7932ff9a37e33ca8577756bec8a1 Mon Sep 17 00:00:00 2001 From: Xiaozhou Li Date: Sat, 2 May 2026 08:46:30 -0700 Subject: [PATCH 1/2] misc improvements - route: stamp RouterUpdated events with block.timestamp (not time.Now) so startup replay doesn't revive stale routers - route: skip periodic refresh when registeredAt==0 to respect explicit deregistration - route: derive event-monitor backtrack from sampled chain block time so the 7-day live-router window is covered on fast chains - server/webapi: rename ospWebapiDefaultPayTimeout -> ...Sec, bump 50 -> 600s (post-blocktime the unit silently shrank from blocks to seconds) - sdk: ResolveCondPayOnChain auto-deploys locally-registered VIRTUAL_CONTRACT conditions before submitting; removes the implicit 'call OnChainGetBooleanOutcome first' prerequisite - docs: drop stale regenerate-legacy-app-bindings.sh reference --- app/appclient.go | 14 +++++ celersdk/pay.go | 17 +++--- client/api.go | 44 +++++++++++++- cnode/cnode.go | 1 + route/controller.go | 115 ++++++++++++++++++++++++++++++----- server/osp_webapi_backend.go | 11 +++- test/e2e/pay_dispute.go | 58 +++++++++++------- tools/scripts/README.md | 2 - 8 files changed, 208 insertions(+), 54 deletions(-) diff --git a/app/appclient.go b/app/appclient.go index dcf6522..7714a95 100644 --- a/app/appclient.go +++ b/app/appclient.go @@ -134,6 +134,20 @@ func (c *AppClient) GetAppChannelDeployedAddr(cid string) (ctype.Addr, error) { return addr, nil } +// EnsureAppChannelDeployed deploys the registered virtual condition contract +// on-chain if it isn't already, and returns the deployed address. Useful for +// callers that need the contract on-chain but don't have a query to run +// (e.g. `PayResolver.resolvePaymentByConditions`, which calls +// `VirtContractResolver.resolve(virtAddr)` and reverts with +// "Nonexistent virtual address" otherwise). +func (c *AppClient) EnsureAppChannelDeployed(cid string) (ctype.Addr, error) { + appChannel := c.GetAppChannel(cid) + if appChannel == nil { + return ctype.ZeroAddr, fmt.Errorf("EnsureAppChannelDeployed: app channel not found") + } + return c.deployIfNeeded(appChannel) +} + // GetBooleanOutcome queries `IBooleanCond.{isFinalized,getOutcome}` for the // registered condition contract, triggering deploy-on-query if the virtual // contract has not been deployed yet. The query bytes are passed through diff --git a/celersdk/pay.go b/celersdk/pay.go index 20bc8de..d9c01a9 100644 --- a/celersdk/pay.go +++ b/celersdk/pay.go @@ -90,14 +90,11 @@ func (mc *Client) RemoveExpiredPays(tk *Token) error { // ResolvePayOnChain settles the payment onchain and receives the payment from OSP. // -// VIRTUAL_CONTRACT prerequisite: PayResolver.resolvePaymentByConditions calls -// VirtContractResolver.resolve(virtAddr) on-chain, which reverts with -// "Nonexistent virtual address" if the virtual condition contract has not been -// deployed yet. The surviving deploy path is the deploy-on-query side effect -// of AppSession.OnChainGetBooleanOutcome — calling it once after registration -// (e.g. with a no-op query) ensures the virtual contract has bytecode by the -// time this resolve tx lands. DEPLOYED_CONTRACT conditions have no such -// prerequisite. +// VIRTUAL_CONTRACT conditions are deployed automatically before the resolve tx +// is submitted: this client walks the pay's conditions and, for any +// VIRTUAL_CONTRACT registered locally via NewAppChannelOnVirtualContract, +// triggers the same deploy path used by OnChainGetBooleanOutcome. Conditions +// registered on a different node are left to that node to deploy. func (mc *Client) ResolvePayOnChain(payID string) error { err := mc.c.ResolveCondPayOnChain(ctype.Hex2PayID(payID)) if err != nil { @@ -144,8 +141,8 @@ func (mc *Client) GetOnChainPaymentInfo(paymentID string) (*OnChainPaymentInfo, } // ResolveIncomingPaymentOnChain submits PayResolver.resolvePaymentByConditions -// for the given payment. See ResolvePayOnChain doc for the VIRTUAL_CONTRACT -// deploy-before-resolve prerequisite — the same applies here. +// for the given payment. Locally-registered VIRTUAL_CONTRACT conditions are +// auto-deployed first (see ResolvePayOnChain doc). func (mc *Client) ResolveIncomingPaymentOnChain(payId string) error { return mc.c.ResolveCondPayOnChain(ctype.Hex2PayID(payId)) } diff --git a/client/api.go b/client/api.go index 022a2aa..bbe3316 100644 --- a/client/api.go +++ b/client/api.go @@ -4,6 +4,7 @@ package client import ( "errors" + "fmt" "math/big" "time" @@ -219,11 +220,52 @@ func (c *CelerClient) SignState(in []byte) []byte { return c.cNode.SignState(in) } -// ResolveCondPayOnChain tries to resolve a payment onchain in the PayRegistry +// ResolveCondPayOnChain tries to resolve a payment onchain in the PayRegistry. +// Before submitting the resolve tx, it walks the pay's `VIRTUAL_CONTRACT` +// conditions and deploys any that are still bytecode-only. PayResolver's +// `resolvePaymentByConditions` calls `VirtContractResolver.resolve(virtAddr)` +// and reverts with "Nonexistent virtual address" if the virtual contract +// hasn't been deployed yet — handling that here makes the resolve API +// self-sufficient instead of silently requiring callers to first invoke +// `OnChainGetBooleanOutcome` for its deploy-on-query side effect. +// +// We can only deploy contracts this node registered locally (the bytecode + +// constructor live in `AppClient.appChannels`); for conditions registered +// elsewhere we trust that the registering side already deployed and let the +// resolve tx fail loudly if not. func (c *CelerClient) ResolveCondPayOnChain(payID ctype.PayIDType) error { + if err := c.ensureVirtualConditionsDeployed(payID); err != nil { + return err + } return c.cNode.Disputer.SettleConditionalPay(payID) } +func (c *CelerClient) ensureVirtualConditionsDeployed(payID ctype.PayIDType) error { + pay, _, found, err := c.cNode.GetDAL().GetPayment(payID) + if err != nil { + return err + } + if !found || pay == nil { + return common.ErrPayNotFound + } + for _, cond := range pay.GetConditions() { + if cond.GetConditionType() != entity.ConditionType_VIRTUAL_CONTRACT { + continue + } + cid := ctype.Bytes2Hex(cond.GetVirtualContractAddress()) + // Locally registered? Trigger deploy-if-needed. Not registered locally + // is the cross-node case — leave it to the registering side; the + // resolve tx will surface a contract revert if neither side deploys. + if c.cNode.AppClient.GetAppChannel(cid) == nil { + continue + } + if _, err := c.cNode.AppClient.EnsureAppChannelDeployed(cid); err != nil { + return fmt.Errorf("ensure virt-contract %s deployed: %w", cid, err) + } + } + return nil +} + func (c *CelerClient) GetCondPayInfoFromRegistry(payID ctype.PayIDType) (*big.Int, uint64, error) { return c.cNode.Disputer.GetCondPayInfoFromRegistry(payID) } diff --git a/cnode/cnode.go b/cnode/cnode.go index 4f30b6d..f52f4ff 100644 --- a/cnode/cnode.go +++ b/cnode/cnode.go @@ -547,6 +547,7 @@ func (c *CNode) initialize( c.nodeConfig, c.masterTransactor, c.monitorService, + c.ethclient, c.dal, c.signer, c.bcastSend, diff --git a/route/controller.go b/route/controller.go index 6a93e70..dc7c273 100644 --- a/route/controller.go +++ b/route/controller.go @@ -3,6 +3,7 @@ package route import ( + "context" "fmt" "math/big" "sync" @@ -25,6 +26,7 @@ import ( "github.com/celer-network/goutils/log" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/ethclient" "google.golang.org/protobuf/proto" ) @@ -35,6 +37,7 @@ type Controller struct { nodeConfig common.GlobalNodeConfig transactor *eth.Transactor monitorService intfs.MonitorService + ethclient *ethclient.Client dal *storage.DAL signer eth.Signer bcastSendCallback BcastSendCallback @@ -65,10 +68,18 @@ const ( checkRegistryInterval = 6 * time.Hour // time interval to check for self-refresh and OSP timeouts refreshIntervalSec = uint64(432000) // 5 days, seconds — refresh self if stored ts is older expireTimeoutSec = uint64(604800) // 7 days, seconds — drop a router whose stored ts is older - // eventBacktrackBlocks is how far back the on-chain event monitor scans on startup so it - // won't miss a still-relevant RouterUpdated event. This stays a block count because the - // event monitor's StartBlock is a block height, independent of contract deadline semantics. - eventBacktrackBlocks = uint64(50000) + // backtrackSafetyMargin is multiplied with `expireTimeoutSec` when computing + // the on-chain event-monitor backtrack window, so a chain whose actual block + // time briefly accelerates can't push a still-live RouterUpdated event out + // of the replay window. + backtrackSafetyMargin = 2 + // minBacktrackBlocks lower-bounds the computed backtrack so we always scan + // at least a sane number of blocks, even on very slow chains. + minBacktrackBlocks = uint64(50000) + // blockTimeSampleSpan is how many blocks back we sample to estimate the + // chain's effective block time at startup. Big enough to absorb per-block + // jitter, small enough to be one HeaderByNumber call. + blockTimeSampleSpan = uint64(1000) routeTTL = 15 ) @@ -78,6 +89,7 @@ func NewController( nodeConfig common.GlobalNodeConfig, transactor *eth.Transactor, monitorService intfs.MonitorService, + ethclient *ethclient.Client, dal *storage.DAL, signer eth.Signer, bcastSendCallback BcastSendCallback, @@ -88,6 +100,7 @@ func NewController( nodeConfig: nodeConfig, transactor: transactor, monitorService: monitorService, + ethclient: ethclient, dal: dal, signer: signer, bcastSendCallback: bcastSendCallback, @@ -155,11 +168,16 @@ func (c *Controller) monitorRouterUpdatedEvent() { txHash := fmt.Sprintf("%x", eLog.TxHash) log.Infoln("Seeing RouterUpdated event, router addr:", routerAddr, "tx hash:", txHash, "callback id:", id, "blkNum:", eLog.BlockNumber) - // The contract stores `block.timestamp` (unix seconds) per router; since the event - // fired in approximately the current block, time.Now() is within a few seconds of - // the canonical contract value. The off-chain comparisons use multi-day windows, so - // this approximation is operationally lossless. - c.processRouterUpdatedEvent(e, uint64(time.Now().Unix())) + // Use the event's block timestamp — that's the canonical + // `block.timestamp` the contract stamped into RouterRegistry. On + // startup replay this can be days old, so falling back to + // time.Now() would silently revive stale routers. + registeredAt, err := c.blockTimestamp(eLog.BlockNumber) + if err != nil { + log.Errorf("fetch block %d timestamp for RouterUpdated event: %s", eLog.BlockNumber, err) + return false + } + c.processRouterUpdatedEvent(e, registeredAt) return false }, ) @@ -201,17 +219,74 @@ func (c *Controller) refreshRouter(routerAddr ctype.Addr, registeredAt uint64) { c.rtBuilder.markOsp(routerAddr, registeredAt) } -// calculates the start block number for event monitor service. -// No matter whether Osp starts from scratch or starts from existing database, -// Osp backtracks eventBacktrackBlocks from the current block so it won't miss -// a still-relevant RouterUpdated event. +// calculateStartBlockNumber computes the on-chain event-monitor start block +// so a RouterUpdated event from anywhere in the live `expireTimeoutSec` +// window is still in range on startup. The block-count backtrack is derived +// from the chain's actual block time (sampled from headers) — a fixed block +// count would either under-cover a fast chain (e.g. 50k blocks ≈ 28h on a +// 2s chain, far less than the 7-day expiry) or waste replay work on a slow +// one. func (c *Controller) calculateStartBlockNumber() *big.Int { currentBlk := c.monitorService.GetCurrentBlockNumber() - backtrack := big.NewInt(0).SetUint64(eventBacktrackBlocks) - if backtrack.Cmp(currentBlk) == 1 { + backtrack := new(big.Int).SetUint64(c.computeBacktrackBlocks(currentBlk)) + if backtrack.Cmp(currentBlk) >= 0 { return big.NewInt(0) } - return currentBlk.Sub(currentBlk, backtrack) // start block number for onchain monitor service + return new(big.Int).Sub(currentBlk, backtrack) +} + +// computeBacktrackBlocks samples the chain's effective block time from a pair +// of header timestamps (current vs ~`blockTimeSampleSpan` blocks back) and +// converts the live-router window into a block count. Falls back to +// `minBacktrackBlocks` if the sample fails or yields a degenerate block time. +func (c *Controller) computeBacktrackBlocks(currentBlk *big.Int) uint64 { + window := expireTimeoutSec * backtrackSafetyMargin + if c.ethclient == nil || currentBlk.Sign() <= 0 { + return minBacktrackBlocks + } + span := new(big.Int).SetUint64(blockTimeSampleSpan) + if span.Cmp(currentBlk) >= 0 { + // Brand-new chain — the window-in-blocks would be hand-wavy; use the + // floor and let the monitor scan from genesis if currentBlk is small. + return minBacktrackBlocks + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + latest, err := c.ethclient.HeaderByNumber(ctx, currentBlk) + if err != nil { + log.Warnf("backtrack block-time sample (latest) failed: %s — using minBacktrackBlocks", err) + return minBacktrackBlocks + } + older, err := c.ethclient.HeaderByNumber(ctx, new(big.Int).Sub(currentBlk, span)) + if err != nil { + log.Warnf("backtrack block-time sample (older) failed: %s — using minBacktrackBlocks", err) + return minBacktrackBlocks + } + if latest.Time <= older.Time { + log.Warnf("backtrack block-time sample non-monotonic (latest %d, older %d) — using minBacktrackBlocks", latest.Time, older.Time) + return minBacktrackBlocks + } + elapsed := latest.Time - older.Time + // blocks needed = window * span / elapsed (avoids float division). + backtrack := window * blockTimeSampleSpan / elapsed + if backtrack < minBacktrackBlocks { + return minBacktrackBlocks + } + return backtrack +} + +// blockTimestamp returns the unix-second timestamp of the given block. +func (c *Controller) blockTimestamp(blockNumber uint64) (uint64, error) { + if c.ethclient == nil { + return 0, fmt.Errorf("ethclient not configured") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + header, err := c.ethclient.HeaderByNumber(ctx, new(big.Int).SetUint64(blockNumber)) + if err != nil { + return 0, err + } + return header.Time, nil } // call routerInfo in router registry contract to check if Osp has been registered. @@ -236,6 +311,14 @@ func (c *Controller) checkAndRefreshIfNeeded() { log.Errorf("query router registry failed: %s", err) return } + // `registeredAt == 0` means RouterRegistry has no entry for us — either + // we were never registered, or we (or the operator) deregistered. Either + // way, refreshing here would silently re-register us; respect the + // deregistration and let an explicit register call bring us back. + if registeredAt == 0 { + log.Warn("OSP not registered on-chain; skipping periodic refresh") + return + } nowTs := uint64(time.Now().Unix()) if nowTs-registeredAt > refreshIntervalSec { c.refreshRouterRegistry() diff --git a/server/osp_webapi_backend.go b/server/osp_webapi_backend.go index 5719e2e..b6e4aca 100644 --- a/server/osp_webapi_backend.go +++ b/server/osp_webapi_backend.go @@ -19,7 +19,14 @@ import ( "google.golang.org/protobuf/types/known/anypb" ) -const ospWebapiDefaultPayTimeout = 50 +// ospWebapiDefaultPayTimeoutSec is the resolve-deadline window used by the +// `SendToken` WebAPI shortcut, which doesn't take a caller-supplied timeout. +// The deadline is `now + this value` (seconds), and must be long enough to +// cover the source→destination forward, the reveal-secret round-trip, and any +// settle-request retry under load. 600s (10 min) gives multi-hop crossnet +// payments comfortable headroom while still being short enough that abandoned +// pays expire promptly. +const ospWebapiDefaultPayTimeoutSec = uint64(600) var _ webapi.OspPayBackend = (*ospWebapiBackend)(nil) @@ -36,7 +43,7 @@ func newOspWebapiBackend(cNode *cnode.CNode) *ospWebapiBackend { } func (b *ospWebapiBackend) SendToken(request *webrpc.SendTokenRequest) (ctype.PayIDType, error) { - return b.sendBooleanPayment(request.GetTokenInfo(), request.GetDestination(), request.GetAmount(), nil, ospWebapiDefaultPayTimeout, request.GetNote()) + return b.sendBooleanPayment(request.GetTokenInfo(), request.GetDestination(), request.GetAmount(), nil, ospWebapiDefaultPayTimeoutSec, request.GetNote()) } func (b *ospWebapiBackend) SendConditionalPayment(request *webrpc.SendConditionalPaymentRequest) (ctype.PayIDType, error) { diff --git a/test/e2e/pay_dispute.go b/test/e2e/pay_dispute.go index c9e21dc..9051ac4 100644 --- a/test/e2e/pay_dispute.go +++ b/test/e2e/pay_dispute.go @@ -38,9 +38,13 @@ // This is the path PayResolver takes when at least one BOOLEAN_AND // condition is finalized-but-false. // -// Two negative scenarios pin down on-chain prerequisites for VIRTUAL_CONTRACT: -// - `runVirtualContractResolveBeforeDeploy`: skipping step 2 reverts with -// "Nonexistent virtual address"; +// Two additional scenarios pin down VIRTUAL_CONTRACT specifics: +// - `runVirtualContractResolveAutoDeploys`: skipping step 2 still resolves +// because the SDK's resolve path auto-deploys any locally-registered +// virtual contract before submitting the resolve tx. This documents the +// deploy-before-resolve invariant the on-chain VirtContractResolver still +// enforces (would revert with "Nonexistent virtual address" otherwise), +// wrapped behind a self-sufficient resolve API. // - `runVirtualContractParallelDeploy`: concurrent deploy-on-query calls // converge on a single deploy tx (the `AppChannel.mu` mutex prevents // duplicate submissions that would revert with VirtContractResolver's @@ -118,10 +122,11 @@ func disputePayWithVirtualContract(t *testing.T, tokenType entity.TokenType, tok t.Error(err) return } - // Negative scenario: resolving a VIRTUAL_CONTRACT pay before the virtual - // contract is deployed must fail at PayResolver. This documents the - // deploy-before-resolve contract enforced on-chain by VirtContractResolver. - if err := runVirtualContractResolveBeforeDeploy(c1, c2, c2EthAddr, tokenType, tokenAddr, + // Resolve auto-deploy scenario: the SDK's `ResolveCondPayOnChain` walks + // the pay's VIRTUAL_CONTRACT conditions and deploys any that are still + // bytecode-only before submitting the resolve tx, so a caller that never + // invokes `OnChainGetBooleanOutcome` still gets a successful resolve. + if err := runVirtualContractResolveAutoDeploys(c1, c2, c2EthAddr, tokenType, tokenAddr, ctype.Hex2Bytes(bytecode), constructor, 4); err != nil { t.Error(err) return @@ -417,13 +422,16 @@ func runVirtualContractFalseOutcomeScenario( return nil } -// runVirtualContractResolveBeforeDeploy registers a virtual condition -// contract (so the off-chain Condition is well-formed) but deliberately skips -// the deploy-on-query step, then sends a conditional pay against that -// undeployed virtual address and asserts that PayResolver rejects the resolve -// with "Nonexistent virtual address". This pins down the on-chain -// deploy-before-resolve contract that the positive scenarios above rely on. -func runVirtualContractResolveBeforeDeploy( +// runVirtualContractResolveAutoDeploys registers a virtual condition +// contract on both clients, sends a conditional pay against it, and resolves +// on-chain WITHOUT calling `GetAppChannelBooleanOutcome` first. The SDK's +// `ResolveCondPayOnChain` walks the pay's VIRTUAL_CONTRACT conditions and +// deploys any locally-registered ones before submitting the resolve tx, so +// the resolve must succeed and amount must be the full sendAmt. Without that +// auto-deploy step the on-chain VirtContractResolver would revert with +// "Nonexistent virtual address" — that revert is the invariant being pinned +// down here, just wrapped behind a self-sufficient resolve API. +func runVirtualContractResolveAutoDeploys( c1, c2 *tf.ClientController, c2EthAddr string, tokenType entity.TokenType, @@ -432,7 +440,7 @@ func runVirtualContractResolveBeforeDeploy( constructor []byte, nonce uint64, ) error { - log.Infof("virtual-contract negative scenario (resolve-before-deploy): nonce=%d", nonce) + log.Infof("virtual-contract resolve auto-deploys scenario: nonce=%d", nonce) appChanID, err := c1.NewAppChannelOnVirtualContract(bytecode, constructor, nonce) if err != nil { @@ -442,9 +450,13 @@ func runVirtualContractResolveBeforeDeploy( return fmt.Errorf("c2 NewAppChannelOnVirtualContract: %w", err) } + // Symmetric pass: argsQueryFinalization=argsQueryOutcome=0x01 so + // `BooleanCondMock.{isFinalized,getOutcome}` both return true and the + // pay registry resolves to the full amount. cond := &entity.Condition{ ConditionType: entity.ConditionType_VIRTUAL_CONTRACT, VirtualContractAddress: ctype.Hex2Bytes(appChanID), + ArgsQueryFinalization: []byte{0x01}, ArgsQueryOutcome: []byte{0x01}, } @@ -461,16 +473,16 @@ func runVirtualContractResolveBeforeDeploy( go tf.AdvanceBlocksUntilDone(done) defer func() { done <- true }() - // Intentionally skip GetAppChannelBooleanOutcome — the virtual contract - // must remain undeployed for this scenario. - _, _, err = c2.SettleConditionalPayOnChain(payID) - if err == nil { - return fmt.Errorf("SettleConditionalPayOnChain unexpectedly succeeded against undeployed virtual contract") + // Intentionally skip GetAppChannelBooleanOutcome — the resolve path is + // expected to deploy the virtual contract on its own. + amount, _, err := c2.SettleConditionalPayOnChain(payID) + if err != nil { + return fmt.Errorf("SettleConditionalPayOnChain (auto-deploy expected): %w", err) } - if !strings.Contains(err.Error(), "Nonexistent virtual address") { - return fmt.Errorf("SettleConditionalPayOnChain error = %v, want substring %q", err, "Nonexistent virtual address") + if amount != sendAmt { + return fmt.Errorf("on-chain pay amount = %s, want %s (auto-deploy resolve)", amount, sendAmt) } - log.Infof("resolve-before-deploy correctly rejected: %v", err) + log.Infof("resolve auto-deploy succeeded: amount=%s", amount) return nil } diff --git a/tools/scripts/README.md b/tools/scripts/README.md index 8ded446..2e84841 100644 --- a/tools/scripts/README.md +++ b/tools/scripts/README.md @@ -34,8 +34,6 @@ Generated-file header convention: The script prepends `// Regenerated by tools/scripts/regenerate-go-bindings.sh — DO NOT EDIT.` as the first line of each output (above abigen's own `Code generated - DO NOT EDIT.` banner) so a future reader who edits a generated file by mistake sees the right script to re-run. The post-AS-D bindings (`app/booleancond.go`, `testing/testapp/booleancondmock.go`) already carry this breadcrumb. The pre-existing chain bindings (`chain/channel-eth-go/...`, `chain/erc20.go`, `route/routerregistry/routerregistry.go`) will pick it up the next time they're regenerated — that produces a one-line diff per file unrelated to whatever else triggered the regen, which is expected. -`regenerate-legacy-app-bindings.sh` does the same thing for its lone survivor. - Default behavior: - looks for a sibling contracts checkout at `../agent-pay-contracts` From 9c810fc3c3edc3c58afaa7a05e2c92a4d50ca0c0 Mon Sep 17 00:00:00 2001 From: Xiaozhou Li Date: Sat, 2 May 2026 11:06:17 -0700 Subject: [PATCH 2/2] mis improvements - app: GetBooleanOutcome short-circuits to (false,false,nil) when isFinalized is false, matching PayResolver - celersdk: drop legacy BooleanCondition / SendTokenWithCondition / SendETHWithCondition / bc2c (no callers; SendConditionalPayment covers) - webapi: document block_duration as seconds in web_api.proto; inline the no-op paymentInfoFromClientPayment wrapper - cleanup comments and docs --- app/appclient.go | 11 ++++++++--- celersdk/appsession.go | 12 +++--------- celersdk/pay.go | 27 --------------------------- celersdk/types.go | 8 -------- celersdk/utils.go | 15 --------------- test/e2e/pay_dispute.go | 11 ++++++----- tools/scripts/README.md | 2 +- webapi/api_server.go | 10 ++-------- webapi/api_server_test.go | 5 ++--- webapi/proto/web_api.proto | 3 +++ 10 files changed, 25 insertions(+), 79 deletions(-) diff --git a/app/appclient.go b/app/appclient.go index 7714a95..dec8bf2 100644 --- a/app/appclient.go +++ b/app/appclient.go @@ -150,9 +150,11 @@ func (c *AppClient) EnsureAppChannelDeployed(cid string) (ctype.Addr, error) { // GetBooleanOutcome queries `IBooleanCond.{isFinalized,getOutcome}` for the // registered condition contract, triggering deploy-on-query if the virtual -// contract has not been deployed yet. The query bytes are passed through -// unchanged (matches what `PayResolver` does on-chain) — no `SessionQuery` -// wrapping. +// contract has not been deployed yet. The same query bytes go to both calls +// (matches what `PayResolver` does on-chain). When `isFinalized` returns +// false `getOutcome` is not called — `PayResolver` short-circuits the same +// way, and a strict `IBooleanCond` is free to revert on `getOutcome` before +// finalization. func (c *AppClient) GetBooleanOutcome(cid string, query []byte) (bool, bool, error) { appChannel := c.GetAppChannel(cid) if appChannel == nil { @@ -170,6 +172,9 @@ func (c *AppClient) GetBooleanOutcome(cid string, query []byte) (bool, bool, err if err != nil { return false, false, fmt.Errorf("contract IsFinalized error: %w", err) } + if !finalized { + return false, false, nil + } result, err := contract.GetOutcome(&bind.CallOpts{}, query) if err != nil { return false, false, fmt.Errorf("contract GetOutcome error: %w", err) diff --git a/celersdk/appsession.go b/celersdk/appsession.go index 09cbe8f..cd80c09 100644 --- a/celersdk/appsession.go +++ b/celersdk/appsession.go @@ -1,14 +1,8 @@ // Copyright 2018-2025 Celer Network -// SDK APIs dealing with app sessions backed by stateless `IBooleanCond` -// virtual condition contracts. -// -// Post-trim the legacy gaming surface (turn-based state-exchange protocol with -// `SignAppData` / `HandleMatchData` / opcodes / seqnum tracking, on-chain -// `applyAction` / introspection / oracle disputes, the `NewAppSessionOnDeployedContract` -// path and its multisession dependencies) is gone. What remains is the thin -// wrapper around `client.CelerClient`'s registration / outcome-query surface -// for VIRTUAL_CONTRACT condition contracts. +// SDK APIs for app sessions backed by stateless `IBooleanCond` virtual +// condition contracts. Thin wrapper around `client.CelerClient`'s +// registration and outcome-query surface for VIRTUAL_CONTRACT conditions. package celersdk diff --git a/celersdk/pay.go b/celersdk/pay.go index d9c01a9..24e3498 100644 --- a/celersdk/pay.go +++ b/celersdk/pay.go @@ -45,33 +45,6 @@ func (mc *Client) SendToken(tk *Token, receiver string, amtWei string, noteTypeU return ret, nil } -func (mc *Client) SendETHWithCondition(receiver string, amtWei string, cond *BooleanCondition) (string, error) { - return mc.SendTokenWithCondition(nil, receiver, amtWei, cond) -} - -// When should we call onSent? or do we need a new callback func? -func (mc *Client) SendTokenWithCondition(tk *Token, receiver string, amtWei string, cond *BooleanCondition) (string, error) { - xfer := createXfer(tk, receiver, amtWei) - timeout := cPayTimeout - if cond.TimeoutSec > 0 { - timeout = cond.TimeoutSec - } - condition, err := bc2c(cond) - if err != nil { - log.Errorln("SendTokenWithCondition:", err) - return ctype.ZeroPayIDHex, err - } - payID, err := mc.c.AddBooleanPay( - xfer, []*entity.Condition{condition}, uint64(time.Now().Unix())+uint64(timeout), nil /*note*/, 0) - if err != nil { - log.Errorln("SendTokenWithCondition:", err) - return ctype.ZeroPayIDHex, err - } - ret := ctype.PayID2Hex(payID) - log.Debugln("Sent pay:", ret) - return ret, nil -} - // ConfirmPay settles the condpay, ie. actually paid to pay dest func (mc *Client) ConfirmPay(payID string) error { return mc.c.ConfirmBooleanPay(ctype.Hex2PayID(payID)) diff --git a/celersdk/types.go b/celersdk/types.go index df89e5c..a44c3ee 100644 --- a/celersdk/types.go +++ b/celersdk/types.go @@ -64,14 +64,6 @@ type Balance struct { ReceivingCap string } -type BooleanCondition struct { - OnChainDeployed bool - OnChainAddress string // on-chain IBooleanCond contract address if OnChainDeployed is true - VirtualContractAddress string // deterministic virtual-contract address (hex) from CreateAppSessionOnVirtualContract; ignored if OnChainDeployed is true - ArgsForQueryOutcome []byte - TimeoutSec int // pay deadline = wall-clock unix time + TimeoutSec -} - type Token struct { Erctype string // ERC20, ERC721 etc. Addr string // token contract addr diff --git a/celersdk/utils.go b/celersdk/utils.go index a394495..d40ffba 100644 --- a/celersdk/utils.go +++ b/celersdk/utils.go @@ -54,21 +54,6 @@ func createXfer(tk *Token, receiver, amtWei string) *entity.TokenTransfer { return xfer } -func bc2c(bc *BooleanCondition) (*entity.Condition, error) { - if bc.OnChainDeployed { - return &entity.Condition{ - ConditionType: entity.ConditionType_DEPLOYED_CONTRACT, - DeployedContractAddress: ctype.Hex2Addr(bc.OnChainAddress).Bytes(), - ArgsQueryOutcome: bc.ArgsForQueryOutcome, - }, nil - } - return &entity.Condition{ - ConditionType: entity.ConditionType_VIRTUAL_CONTRACT, - VirtualContractAddress: ctype.Hex2Bytes(bc.VirtualContractAddress), - ArgsQueryOutcome: bc.ArgsForQueryOutcome, - }, nil -} - func sdkToken2entityToken(tk *Token) *entity.TokenInfo { var token *entity.TokenInfo if tk == nil { // ETH case diff --git a/test/e2e/pay_dispute.go b/test/e2e/pay_dispute.go index 9051ac4..2b07844 100644 --- a/test/e2e/pay_dispute.go +++ b/test/e2e/pay_dispute.go @@ -1,14 +1,15 @@ // Copyright 2018-2025 Celer Network -// Conditional-pay dispute coverage. After AS-B trimmed the gaming -// state-machine machinery, the surviving "dispute" path is just: +// Conditional-pay dispute coverage. The dispute path is: // // 1. send a conditional pay whose Condition references an IBooleanCond // contract (either VIRTUAL_CONTRACT bytecode registered off-chain or // a DEPLOYED_CONTRACT address); -// 2. for VIRTUAL_CONTRACT only, ensure the contract is on-chain by -// calling `GetBooleanOutcomeForAppSession` (the surviving deploy-on- -// query path through `AppClient.deployIfNeeded`); +// 2. for VIRTUAL_CONTRACT only, ensure the contract is on-chain — either +// by calling `GetBooleanOutcomeForAppSession` (deploy-on-query through +// `AppClient.deployIfNeeded`) or by relying on `ResolveCondPayOnChain` +// to auto-deploy locally-registered conditions before submitting the +// resolve tx; // 3. invoke `PayResolver.resolvePaymentByConditions`, which calls // `IBooleanCond.isFinalized(argsQueryFinalization)` and (only if true) // `IBooleanCond.getOutcome(argsQueryOutcome)` on the deployed diff --git a/tools/scripts/README.md b/tools/scripts/README.md index 2e84841..65b1b8a 100644 --- a/tools/scripts/README.md +++ b/tools/scripts/README.md @@ -32,7 +32,7 @@ Handwritten exception: Generated-file header convention: -The script prepends `// Regenerated by tools/scripts/regenerate-go-bindings.sh — DO NOT EDIT.` as the first line of each output (above abigen's own `Code generated - DO NOT EDIT.` banner) so a future reader who edits a generated file by mistake sees the right script to re-run. The post-AS-D bindings (`app/booleancond.go`, `testing/testapp/booleancondmock.go`) already carry this breadcrumb. The pre-existing chain bindings (`chain/channel-eth-go/...`, `chain/erc20.go`, `route/routerregistry/routerregistry.go`) will pick it up the next time they're regenerated — that produces a one-line diff per file unrelated to whatever else triggered the regen, which is expected. +The script prepends `// Regenerated by tools/scripts/regenerate-go-bindings.sh — DO NOT EDIT.` as the first line of each output (above abigen's own `Code generated - DO NOT EDIT.` banner) so a future reader who edits a generated file by mistake sees the right script to re-run. Files regenerated through this path carry the breadcrumb; any not-yet-regenerated file picks it up the next time the script runs, which produces a one-line diff per file unrelated to whatever else triggered the regen. Default behavior: diff --git a/webapi/api_server.go b/webapi/api_server.go index 670178a..486fb06 100644 --- a/webapi/api_server.go +++ b/webapi/api_server.go @@ -524,16 +524,10 @@ func (s *ApiServer) GetOnChainPaymentInfo( return &rpc.OnChainPaymentInfo{Amount: info.Amount, ResolveDeadline: info.ResolveDeadline}, nil } -// Keep the legacy helper name as a thin wrapper so the client WebAPI path and -// the OSP WebAPI path share one PaymentInfo mapping implementation. -func paymentInfoFromClientPayment(payment *celersdkintf.Payment) *rpc.PaymentInfo { - return paymentInfoFromPayment(payment) -} - func (s *ApiServer) SubscribeIncomingPayments( empty *empty.Empty, stream rpc.WebApi_SubscribeIncomingPaymentsServer) error { writeToStream := func(payment *celersdkintf.Payment) error { - return stream.Send(paymentInfoFromClientPayment(payment)) + return stream.Send(paymentInfoFromPayment(payment)) } callbackImpl := s.callbackImpl for { @@ -586,7 +580,7 @@ func (s *ApiServer) GetIncomingPaymentInfo( if err != nil { return nil, err } - return paymentInfoFromClientPayment(payment), nil + return paymentInfoFromPayment(payment), nil } func (s *ApiServer) GetOutgoingPaymentStatus( diff --git a/webapi/api_server_test.go b/webapi/api_server_test.go index be131b8..ff9f47c 100644 --- a/webapi/api_server_test.go +++ b/webapi/api_server_test.go @@ -47,9 +47,8 @@ func TestGetAppSessionAfterDelete(t *testing.T) { } // Public-handler regression: a query for an unknown session ID must return -// codes.NotFound at the gRPC surface (rather than nil-deref panicking, the -// pre-fix behavior). Exercises the actual surviving handlers, not just the -// helper they delegate to. +// codes.NotFound at the gRPC surface (rather than nil-deref panicking). +// Exercises the actual public handler, not just the helper it delegates to. func TestGetDeployedAddressForAppSessionUnknownID(t *testing.T) { s := &ApiServer{ appSessionMap: make(map[string]*celersdk.AppSession), diff --git a/webapi/proto/web_api.proto b/webapi/proto/web_api.proto index cb08b54..63c8d4e 100644 --- a/webapi/proto/web_api.proto +++ b/webapi/proto/web_api.proto @@ -29,6 +29,9 @@ message TokenInfo { } message SetDelegationRequest{ repeated TokenInfo token_infos = 1; + // Delegation expiry, interpreted as a number of seconds added to the + // current unix time. The field name is kept for wire compatibility with + // existing clients; the value is no longer in blocks. int64 block_duration = 2; } message OpenPaymentChannelRequest {