SEP-29 as implemented in JS/GO SDK's #78
Conversation
JakeUrban
requested review from
leighmcculloch and
abuiles
and removed request for
leighmcculloch
| @@ -427,4 +427,140 @@ | |||
| end | |||
| end | |||
|
|
|||
| describe "#submit_transaction" do | |||
There was a problem hiding this comment.
All the tests for submit_transaction involve operations that require memos. Can we add a test that has a mix of operations, some that require memo and others that don't, and a test that has only operations that don't require a memo and no memo is set to ensure the error is not raised in that case?
Co-Authored-By: Leigh McCulloch <leigh@mcchouse.com>
lib/stellar/client.rb
Outdated
| @@ -3,7 +3,7 @@ | |||
| require 'securerandom' | |||
|
|
|||
| module Stellar | |||
| class InvalidSep10ChallengeError < StandardError; end | |||
| class AccountRequiresMemoError < StandardError; end | |||
There was a problem hiding this comment.
@JakeUrban can you add the accountId and the operationIndex similar to https://github.com/stellar/js-stellar-sdk/blob/master/src/errors.ts#L90?
|
@abuiles @leighmcculloch this PR should be ready for another look when you get a chance |
| @@ -244,7 +252,7 @@ def check_memo_required(tx_envelope) | |||
| end | |||
| if info.data["config.memo_required"] == "MQ==" | |||
| # MQ== is the base64 encoded string for the string "1" | |||
| raise AccountRequiresMemoError.new("account requires memo") | |||
| raise AccountRequiresMemoError.new("account requires memo", destination, idx) | |||
There was a problem hiding this comment.
@JakeUrban nice! can you update the test to assert this and also update the demo code so people know what attrs they can work with?
There was a problem hiding this comment.
When you say demo code you're referring to the /examples files right? I don't want to update the examples before the functionality is released. I think doing so would confuse people.
But I'll add that test.
resolves #75