Skip to content

Commit

Permalink
Add a simple API that lets you tag wallets with arbitrary string->byt…
Browse files Browse the repository at this point in the history
…e[] pairs.
  • Loading branch information
mikehearn committed May 8, 2014
1 parent c277dc7 commit 268dfe2
Show file tree
Hide file tree
Showing 8 changed files with 1,144 additions and 19 deletions.
14 changes: 13 additions & 1 deletion core/src/bitcoin.proto
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,16 @@ message Extension {
required bool mandatory = 3;
}

/**
* A simple key->value mapping that has no interpreted content at all. A bit like the extensions mechanism except
* an extension is keyed by the ID of a piece of code that's loaded with the given data, and has the concept of
* being mandatory if that code isn't found. Whereas this is just a blind key/value store.
*/
message Tag {
required string tag = 1;
required bytes data = 2;

This comment has been minimized.

Copy link
@schildbach

schildbach May 13, 2014

Member

I would maybe make this field (data) optional. Reason: In future, we might add other data types, like Strings or our new monatary value type.

This comment has been minimized.

Copy link
@mikehearn

mikehearn May 13, 2014

Author Member

Well, those can all be represented as bytes obviously :-) I'm not sure this API should get too powerful. Like I said, for many uses, a better approach is to use a real wallet format extension. It's not very difficult.

}

/** A bitcoin wallet */
message Wallet {
/**
Expand Down Expand Up @@ -289,5 +299,7 @@ message Wallet {
// can be used to recover a compromised wallet, or just as part of preventative defence-in-depth measures.
optional uint64 key_rotation_time = 13;

// Next tag: 16
repeated Tag tags = 16;

// Next tag: 17
}
5 changes: 4 additions & 1 deletion core/src/main/java/com/google/bitcoin/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.bitcoin.script.ScriptChunk;
import com.google.bitcoin.store.UnreadableWalletException;
import com.google.bitcoin.store.WalletProtobufSerializer;
import com.google.bitcoin.utils.BaseTaggableObject;
import com.google.bitcoin.utils.ListenerRegistration;
import com.google.bitcoin.utils.Threading;
import com.google.bitcoin.wallet.*;
Expand All @@ -37,6 +38,7 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import com.google.protobuf.ByteString;
import org.bitcoinj.wallet.Protos.Wallet.EncryptionType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -98,7 +100,7 @@
* {@link Wallet#autosaveToFile(java.io.File, long, java.util.concurrent.TimeUnit, com.google.bitcoin.wallet.WalletFiles.Listener)}
* for more information about this.</p>
*/
public class Wallet implements Serializable, BlockChainListener, PeerFilterProvider {
public class Wallet extends BaseTaggableObject implements Serializable, BlockChainListener, PeerFilterProvider {
private static final Logger log = LoggerFactory.getLogger(Wallet.class);
private static final long serialVersionUID = 2L;
private static final int MINIMUM_BLOOM_DATA_LENGTH = 8;
Expand Down Expand Up @@ -205,6 +207,7 @@ public Wallet(NetworkParameters params) {
extensions = new HashMap<String, WalletExtension>();
// Use a linked hash map to ensure ordering of event listeners is correct.
confidenceChanged = new LinkedHashMap<Transaction, TransactionConfidence.Listener.ChangeReason>();
tags = new HashMap<String, ByteString>();
createTransientState();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ public Protos.Wallet walletToProto(Wallet wallet) {

populateExtensions(wallet, walletBuilder);

for (Map.Entry<String, ByteString> entry : wallet.getTags().entrySet()) {
Protos.Tag.Builder tag = Protos.Tag.newBuilder().setTag(entry.getKey()).setData(entry.getValue());
walletBuilder.addTags(tag);
}

// Populate the wallet version.
walletBuilder.setVersion(wallet.getVersion());

Expand Down Expand Up @@ -476,6 +481,10 @@ public void readWallet(Protos.Wallet walletProto, Wallet wallet) throws Unreadab

loadExtensions(wallet, walletProto);

for (Protos.Tag tag : walletProto.getTagsList()) {
wallet.setTag(tag.getTag(), tag.getData());
}

if (walletProto.hasVersion()) {
wallet.setVersion(walletProto.getVersion());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.google.bitcoin.utils;

import com.google.common.collect.Maps;
import com.google.protobuf.ByteString;

import javax.annotation.Nullable;
import java.util.HashMap;
import java.util.Map;

import static com.google.common.base.Preconditions.checkNotNull;

/**
* A simple implementation of {@link TaggableObject} that just uses a lazily created hashmap that is
* synchronized on this objects Java monitor.
*/
public class BaseTaggableObject implements TaggableObject {
@Nullable protected Map<String, ByteString> tags;

/** {@inheritDoc} */
@Override
@Nullable
public synchronized ByteString maybeGetTag(String tag) {
if (tags == null)
return null;
else
return tags.get(tag);
}

/** {@inheritDoc} */
@Override
public ByteString getTag(String tag) {
ByteString b = maybeGetTag(tag);
if (b == null)
throw new IllegalArgumentException("Unknown tag " + tag);
return b;
}

/** {@inheritDoc} */
@Override
public synchronized void setTag(String tag, ByteString value) {

This comment has been minimized.

Copy link
@schildbach

schildbach May 13, 2014

Member

Hmmm, I don't see how this method autosaves the wallet.

This comment has been minimized.

Copy link
@mikehearn

mikehearn May 13, 2014

Author Member

Good point! That looks like a bug.

checkNotNull(tag);
checkNotNull(value);
if (tags == null)
tags = new HashMap<String, ByteString>();
tags.put(tag, value);
}

/** {@inheritDoc} */
@Override
public synchronized Map<String, ByteString> getTags() {
if (tags != null)
return Maps.newHashMap(tags);
else
return Maps.newHashMap();
}
}
36 changes: 36 additions & 0 deletions core/src/main/java/com/google/bitcoin/utils/TaggableObject.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.google.bitcoin.utils;

import com.google.protobuf.ByteString;

import javax.annotation.Nullable;
import java.util.Map;

/**
* <p>An object that can carry around and possibly serialize a map of strings to immutable byte arrays. Tagged objects
* can have data stored on them that might be useful for an application developer. For example a wallet can store tags,
* and thus this would be a reasonable place to put any important data items that the bitcoinj API does not allow for:
* things like exchange rates at the time a transaction was made would currently fall into this category. Of course,
* it helps interop and other developers if you introduce a real type safe API for a new feature instead of using this
* so please consider that path, if you find yourself tempted to store tags!</p>
*
* <p>Good tag names won't conflict with other people's code, should you one day decide to merge them. Choose tag names
* like "com.example:keyowner:02b7e6dc316dfaa19c5a599f63d88ffeae398759b857ca56b2f69de3e815381343" instead of
* "owner" or just "o". Also, it's good practice to create constants for each string you use, to help avoid typos
* in string parameters causing confusing bugs!</p>
*/
public interface TaggableObject {
/** Returns the immutable byte array associated with the given tag name, or null if there is none. */
@Nullable ByteString maybeGetTag(String tag);

/**
* Returns the immutable byte array associated with the given tag name, or throws {@link java.lang.IllegalArgumentException}
* if that tag wasn't set yet.
*/
ByteString getTag(String tag);

This comment has been minimized.

Copy link
@schildbach

schildbach May 13, 2014

Member

Why polluting this interface with Protobuf (ByteString) types? I'm for using good old byte arrays. (As documented, by the way.)

This comment has been minimized.

Copy link
@mikehearn

mikehearn May 13, 2014

Author Member

ByteString basically is a byte array, it's just immutable. The tags are stuffed directly into the map, so if we used byte[] then either we'd need to manually copy (perhaps pointlessly) or else this code would be buggy:

byte[] bits = ....;
wallet.setTag("foo.bar", bits);
loadData(bits);
wallet.setTag("baz", bits);

// both tags are now the same content despite how the code looks.


/** Associates the given immutable byte array with the string tag. See the docs for TaggableObject to learn more. */
void setTag(String tag, ByteString value);

/** Returns a copy of all the tags held by this wallet. */

This comment has been minimized.

Copy link
@schildbach

schildbach May 13, 2014

Member

Wallet? Doesn't make sense for a generic type -- what if I tag an address? Guess it should read "object".

This comment has been minimized.

Copy link
@mikehearn

mikehearn May 13, 2014

Author Member

Right, refactoring glitch. Will fix.

public Map<String, ByteString> getTags();
}

0 comments on commit 268dfe2

Please sign in to comment.