Stellar integration#4858
Conversation
Oghma
requested review from
SEJeff,
bemic,
djb15,
evan-gray,
johnsaigle,
kcsongor,
mdulin2,
panoel and
pleasew8t
as code owners
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
Hi @Oghma, I'd recommend squashing the commits above in order to have an easier time with CI going forward. As of now, the GitHub front-end won't allow us to run CI against this PR due to conflicts. As time goes on, this is likely to reoccur as this large PR is issued against 
|
…F message types to distinguish from integrator emitted messages (wormhole-foundation#4865)
Co-authored-by: djb15 <djb15@users.noreply.github.com>
|
Hi @johnsaigle. I squashed in a single commit and rebased it on top of main |
Adds the Stellar core Wormhole contract (Soroban), the Stellar watcher in the guardian node, and the Tilt/devnet wiring for local Stellar testing. Squashes the full Stellar integration work into a single commit. Co-authored-by: Goutham <dkgoutham1806@gmail.com> Co-authored-by: Marko Cupic <71461781+cupicmarko@users.noreply.github.com> Co-authored-by: Stavros Vlachakis <89769224+svlachakis@users.noreply.github.com> Signed-off-by: Matteo Lisotto <matteo.lisotto@gmail.com>
| } | ||
|
|
||
| // parseMessageFromXDR parses the XDR-encoded Soroban event value into a MessagePublication. | ||
| func parseMessageFromXDR(data []byte, logger *zap.Logger) *common.MessagePublication { |
There was a problem hiding this comment.
I think it makes sense to have this as a method on the watcher rather than a free-floating function. This would allow you to use w.chainId for emitter chain below rather than using Unset which will always be invalid.
| Timestamp: time.Time{}, | ||
| Nonce: nonce, | ||
| Sequence: sequence, | ||
| ConsistencyLevel: uint8(consistencyLevel), |
There was a problem hiding this comment.
The uint32 value is truncated to uint8 without checking for overflow. Could you add a check somewhere that returns an error if the consistency level is > MaxUint8?
| var sequence uint64 | ||
| var emitterAddress []byte | ||
| var payload []byte | ||
| var consistencyLevel uint32 |
There was a problem hiding this comment.
| var consistencyLevel uint32 | |
| var consistencyLevel uint8 |
Better to use the correct type here and ensure that the JSON value fits inside of it
| copy(emitter[:], emitterAddress) | ||
| } | ||
|
|
||
| return &common.MessagePublication{ |
There was a problem hiding this comment.
Maybe t would be better to return only the parsed values here instead of a partially-constructed MessagePublication?
IMO it's difficult to audit whether this struct is being created correctly when its fields are getting set in different places. I'm not sure passing around a semantically invalid Message Publication struct is the right way to go. (Empty values for TxID, Timestamp, and EmitterChain aren't valid)
| mp.EmitterChain = w.chainID | ||
| mp.IsReobservation = true | ||
|
|
||
| logger.Info("reobserved stellar message", |
There was a problem hiding this comment.
Can you change this to use the ZapFields method instead? It allows us to get a common format when logging MessagePublicatons
wormhole/node/pkg/common/chainlock.go
Line 689 in 87111a6
| {EmitterChainID: vaa.ChainIDMonad, DailyLimit: 500_000, BigTransactionSize: 50_000}, | ||
| {EmitterChainID: vaa.ChainIDMegaETH, DailyLimit: 10_000, BigTransactionSize: 10_000}, | ||
| {EmitterChainID: vaa.ChainIDZeroGravity, DailyLimit: 10_000, BigTransactionSize: 10_000}, | ||
| {EmitterChainID: vaa.ChainIDZeroGravity, DailyLimit: 50_000, BigTransactionSize: 10_000}, |
There was a problem hiding this comment.
This looks like a mistake, maybe a rebase issue? Same with the mainnet tokens changes. I don't believe this PR should modify the governor package.
| // the raw account ID as their emitter (the 20-byte account left-padded with 12 zero bytes), so | ||
| // prepending this non-zero prefix makes it impossible to forge a generated message by sending a | ||
| // payment to the core account. The 20-byte account ID remains recoverable in the last 20 bytes. | ||
| var generatedEmitterPrefix = [4]byte{'X', 'R', 'P', 'L'} |
There was a problem hiding this comment.
It also looks like this is included from #4865 by mitake
5 participants
Summary
Adds Stellar/Soroban support to Wormhole: a complete Soroban implementation of
the Wormhole Core contract, supporting client libraries, and a guardian node
watcher for the Stellar chain.
What's included
Soroban contracts (
stellar/)contracts/wormhole-contract) — message posting,guardian set management, message fee handling, and the full governance action
set (guardian set upgrade, contract upgrade, set message fee, transfer fees),
plus VAA parsing/verification and
__constructorinitialization.Soroban client library (
contracts/wormhole-soroban-client) — sharedtypes, errors, constants, and a
BytesReaderfor VAA decoding. This is thelibrary users import to interact with the Wormhole smart contract.
Guardian node watcher (
node/)node/pkg/watchers/stellar/) with unit tests.guardiandvianode/cmd/guardiand/node.go.Tooling & tests
stellar/integration-tests,contracts/wormhole-contract/src/tests) covering initialize/query,post message, message fee, transfer fees, guardian set upgrade, and contract
upgrade flows.
stellar/scripts).Tiltfile,stellar/stellar.yaml).stellar/ARCHITECTURE.md,stellar/README.md).Notes
node/go.mod/go.sumupdated for the Stellar SDK dependencies.