Skip to content

Commit

Permalink
all: Revert SEP23 (Muxed Account strkeys) (#2612)
Browse files Browse the repository at this point in the history
* Revert SEP23 (Muxed Account strkeys)

This change completely removes support for M-strkeys.

After this change:

1. There is no way for users of the Go SDK to create or parse M-strkeys.  In
   particular they cannot provide M-strkeys to create a transaction and in order
   to get the strkey of `MuxedAccount` they first need to transform it to an
   `AccountId` leading to a G-strkey (i.e.  the memo id being is dropped).

2. Users **can** still tinker with `MuxedAccounts` using the XDR representation,
   of transactions (using the using the generated code in the `xdr` package).

* Reinstate disallowing muxed accounts in ReadChallengeTx
  • Loading branch information
2opremio authored May 21, 2020
1 parent 6cdb4e8 commit 72b49a6
Show file tree
Hide file tree
Showing 37 changed files with 223 additions and 613 deletions.
14 changes: 4 additions & 10 deletions clients/horizonclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/stellar/go/xdr"
"io"
"net/http"
"net/url"
Expand All @@ -17,6 +16,7 @@ import (
"github.com/stellar/go/txnbuild"

"github.com/manucorporat/sse"

hProtocol "github.com/stellar/go/protocols/horizon"
"github.com/stellar/go/protocols/horizon/effects"
"github.com/stellar/go/protocols/horizon/operations"
Expand Down Expand Up @@ -64,16 +64,10 @@ func (c *Client) checkMemoRequired(transaction *txnbuild.Transaction) error {
continue
}

xdr.MustMuxedAccountAddress(destination)
muxed, err := xdr.AddressToMuxedAccount(destination)
if err != nil {
return errors.Wrapf(err, "destination %v is not a valid address", destination)
}
// Skip destination addresses with a memo id because the address has a memo
// encoded within it
destinationHasMemoID := muxed.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519
// TODO: once we support M-strkeys (SEP23), also check whether the destination
// is a muxed account with a memo ID.

if destinations[destination] || destinationHasMemoID {
if destinations[destination] {
continue
}
destinations[destination] = true
Expand Down
25 changes: 0 additions & 25 deletions clients/horizonclient/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -917,31 +917,6 @@ func TestSubmitTransactionXDRRequest(t *testing.T) {
}
}

func TestCheckMemoRequiredSkipsMemoIDAddress(t *testing.T) {
client := &Client{HorizonURL: "https://localhost/"}

kp := keypair.MustParseFull("SA26PHIKZM6CXDGR472SSGUQQRYXM6S437ZNHZGRM6QA4FOPLLLFRGDX")
sourceAccount := txnbuild.NewSimpleAccount(kp.Address(), int64(0))

payment := txnbuild.Payment{
Destination: "MCAAAAAAAAAAAAB7BQ2L7E5NBWMXDUCMZSIPOBKRDSBYVLMXGSSKF6YNPIB7Y77ITKNOG",
Amount: "10",
Asset: txnbuild.NativeAsset{},
}

tx, err := txnbuild.NewTransaction(
txnbuild.TransactionParams{
SourceAccount: &sourceAccount,
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{&payment},
BaseFee: txnbuild.MinBaseFee,
Timebounds: txnbuild.NewTimebounds(0, 10),
},
)
assert.NoError(t, err)
assert.NoError(t, client.checkMemoRequired(tx))
}

func TestSubmitTransactionRequest(t *testing.T) {
hmock := httptest.NewClient()
client := &Client{
Expand Down
5 changes: 3 additions & 2 deletions exp/services/webauth/internal/serve/challenge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ func TestChallenge(t *testing.T) {
require.NoError(t, err)

assert.Len(t, tx.Signatures(), 1)
sourceAccount := tx.SourceAccount()
sourceAccount := tx.SourceAccount().ToAccountId()
assert.Equal(t, serverKey.Address(), sourceAccount.Address())
assert.Equal(t, tx.SeqNum(), int64(0))
assert.Equal(t, time.Unix(int64(tx.TimeBounds().MaxTime), 0).Sub(time.Unix(int64(tx.TimeBounds().MinTime), 0)), time.Minute)
assert.Len(t, tx.Operations(), 1)
assert.Equal(t, account.Address(), tx.Operations()[0].SourceAccount.Address())
opSourceAccount := tx.Operations()[0].SourceAccount.ToAccountId()
assert.Equal(t, account.Address(), opSourceAccount.Address())
assert.Equal(t, xdr.OperationTypeManageData, tx.Operations()[0].Body.Type)
assert.Regexp(t, "^testserver auth", tx.Operations()[0].Body.ManageDataOp.DataName)

Expand Down
3 changes: 2 additions & 1 deletion network/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ func TestHashTransaction(t *testing.T) {
_, err = HashTransactionInEnvelope(txe, "")
assert.Contains(t, err.Error(), "empty network passphrase")

sourceAID := xdr.MustAddress("GCLOMB72ODBFUGK4E2BK7VMR3RNZ5WSTMEOGNA2YUVHFR3WMH2XBAB6H")
feeBumpTx := xdr.FeeBumpTransaction{
Fee: 123456,
FeeSource: xdr.MustMuxedAccountAddress("GCLOMB72ODBFUGK4E2BK7VMR3RNZ5WSTMEOGNA2YUVHFR3WMH2XBAB6H"),
FeeSource: sourceAID.ToMuxedAccount(),
InnerTx: xdr.FeeBumpTransactionInnerTx{
Type: xdr.EnvelopeTypeEnvelopeTypeTx,
V1: &xdr.TransactionV1Envelope{
Expand Down
2 changes: 2 additions & 0 deletions services/horizon/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ file. This project adheres to [Semantic Versioning](https://semver.org/).

## Unreleased

* Drop support for MuxedAccounts strkeys (spec'ed in [SEP23](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0023.md)).
SEP23 is still a draft and we don't want to encourage storing strkeys which may not be definite.
* Replace `SequenceProvider` implementation with one which queries the Horizon DB for sequence numbers instead of the Stellar Core DB.

## v1.3.0
Expand Down
3 changes: 2 additions & 1 deletion services/horizon/internal/actions_transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ func TestTransactionActions_PostSuccessful(t *testing.T) {
ht := StartHTTPTest(t, "failed_transactions")
defer ht.Finish()

destAID := xdr.MustAddress("GBXGQJWVLWOYHFLVTKWV5FGHA3LNYY2JQKM7OAJAUEQFU6LPCSEFVXON")
tx2 := xdr.TransactionEnvelope{
Type: xdr.EnvelopeTypeEnvelopeTypeTxV0,
V0: &xdr.TransactionV0Envelope{
Expand All @@ -304,7 +305,7 @@ func TestTransactionActions_PostSuccessful(t *testing.T) {
Body: xdr.OperationBody{
Type: xdr.OperationTypePayment,
PaymentOp: &xdr.PaymentOp{
Destination: xdr.MustMuxedAccountAddress("GBXGQJWVLWOYHFLVTKWV5FGHA3LNYY2JQKM7OAJAUEQFU6LPCSEFVXON"),
Destination: destAID.ToMuxedAccount(),
Asset: xdr.Asset{
Type: xdr.AssetTypeAssetTypeCreditAlphanum4,
AlphaNum4: &xdr.AssetAlphaNum4{
Expand Down
10 changes: 9 additions & 1 deletion services/horizon/internal/db2/core/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,15 @@ func TestSignatures(t *testing.T) {
}

func TestTransaction_SourceAddress_MuxedAccount(t *testing.T) {
muxed := xdr.MustMuxedAccountAddress("MCAAAAAAAAAAAAB7BQ2L7E5NBWMXDUCMZSIPOBKRDSBYVLMXGSSKF6YNPIB7Y77ITKNOG")
aid := xdr.MustAddress("GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ")

muxed := xdr.MuxedAccount{
Type: xdr.CryptoKeyTypeKeyTypeMuxedEd25519,
Med25519: &xdr.MuxedAccountMed25519{
Id: 0xcafebabe,
Ed25519: *aid.Ed25519,
},
}
var tx Transaction
tx.Envelope = xdr.TransactionEnvelope{
Type: xdr.EnvelopeTypeEnvelopeTypeTx,
Expand Down
4 changes: 2 additions & 2 deletions services/horizon/internal/db2/history/fee_bump_scenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ func FeeBumpScenario(tt *test.T, q *Q, successful bool) FeeBumpFixture {
tt.Assert.NoError(insertBuilder.Add(normalTransaction, sequence))
tt.Assert.NoError(insertBuilder.Exec())

account := fixture.Envelope.SourceAccount()
feeBumpAccount := fixture.Envelope.FeeBumpAccount()
account := fixture.Envelope.SourceAccount().ToAccountId()
feeBumpAccount := fixture.Envelope.FeeBumpAccount().ToAccountId()

opBuilder := q.NewOperationBatchInsertBuilder(1)
details, err := json.Marshal(map[string]string{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,8 @@ func TestTransactionToMap_muxed(t *testing.T) {
result, err := transactionToMap(tx, 20)
assert.NoError(t, err)

assert.NotEqual(t, innerSource.Address(), result["account"])
assert.Equal(t, innerAccountID.Address(), result["account"])

assert.NotEqual(t, feeSource.Address(), result["fee_account"])
assert.Equal(t, feeSourceAccountID.Address(), result["fee_account"])

}
3 changes: 2 additions & 1 deletion services/horizon/internal/expingest/fake_ledger_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,12 @@ func (f fakeLedgerBackend) GetLedger(sequence uint32) (bool, ledgerbackend.Ledge
},
},
}
aid := xdr.MustAddress("GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A")
ledgerCloseMeta.TransactionEnvelope[i] = xdr.TransactionEnvelope{
Type: xdr.EnvelopeTypeEnvelopeTypeTx,
V1: &xdr.TransactionV1Envelope{
Tx: xdr.Transaction{
SourceAccount: xdr.MustMuxedAccountAddress("GAHK7EEG2WWHVKDNT4CEQFZGKF2LGDSW2IVM4S5DP42RBW3K6BTODB4A"),
SourceAccount: aid.ToMuxedAccount(),
},
},
}
Expand Down
Loading

0 comments on commit 72b49a6

Please sign in to comment.